DOC: document runtime environment variables in a dedicated page by neutrinoceros · Pull Request #17932 · astropy/astropy

neutrinoceros · 2025-03-25T18:25:37Z

Description

This is split from #17640 at @eerovaher's request.

By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

github-actions · 2025-03-25T18:26:23Z

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

Do the proposed changes actually accomplish desired goals?
Do the proposed changes follow the Astropy coding guidelines?
Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

github-actions · 2025-03-25T18:26:25Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

eerovaher · 2025-03-25T18:45:07Z

docs/development/testguide.rst

 Furthermore, it is possible to change the location of the cache directory
-for the duration of the test run by setting the ``XDG_CACHE_HOME``
-environment variable.
+for the duration of the test run via environment variables, see
+:ref:`environment_variables`.


That's not how it works.

I'm not 100% I'm following. This section is about the test runner. Are you hinting at how conftest.py obliterates user-set XDG_CACHE_HOME values ?

This section is about the test runner.

Irrelevant.

Are you hinting at how conftest.py obliterates user-set XDG_CACHE_HOME values ?

Yes.

Let me rephrase: this section is providing insight on how to use the test runner downstream. In this context, our conftest.py files are not used.

The paragraph is indeed true. I really should have known better because I am the author of the commit that (unintentionally) made our test runner behave like that: 433a534

docs/environment_variables.rst

eerovaher · 2025-03-25T18:54:47Z

docs/environment_variables.rst

+.. note::
+    ``XDG_CONFIG_HOME`` and ``XDG_CACHE_HOME`` come from a Linux-centric
+    specification
+    (see `here <https://wiki.archlinux.org/index.php/XDG_Base_Directory_support>`_


Why is the link pointing to ArchWiki instead of the actual specification?

Your guess is as good as mine.
my two cents: I think it's nice to point to a summary instead, especially when the actual specification is one obvious click away.

It is always better to refer to the original source. Besides, it's a short specification.

I feel it's out of scope to change it here. Can we please postpone this ?

eerovaher · 2025-03-25T18:55:21Z

docs/utils/data.rst

-The ``astropy`` cache is stored in a centralized place (on Linux machines by
-default it is ``$HOME/.astropy/cache``; see :ref:`astropy_config` for
-more details).  You can check its location on your machine::
+The ``astropy`` cache is stored in a centralized place (see :ref:`astropy_config`


Why should the cache documentation point to config documentation?

Because cache location is configurable ? in any case, this is not from this PR.

The cache location is not configurable using astropy configuration items.

Again, not from this PR. I'm already creating potentially massive merge conflicts for myself to handle once we finally circle back to #17640, I would really like for this to stay on topic.

On current main this documentation has to refer to the config documentation because that's where the XDG_CACHE_HOME environment variable is described. If this pull request creates a separate documentation page for the environment variables then there's no reason for the paragraph here to refer to config documentation anymore.

fair enough

eerovaher · 2025-03-25T20:53:54Z

docs/environment_variables.rst

+.. note::
+    ``XDG_CONFIG_HOME`` and ``XDG_CACHE_HOME`` come from a Linux-centric
+    specification


We should emphasize that astropy has only adopted the names of these two environment variables from the specification and nothing else.

the spec is specifically about UNIX-based systems though. I don't think it's a good idea to leave Windows users in the dark, especially because we do support them, so I don't agree with this take.

The updated descriptions in the glossary are clear enough so that this note doesn't have to be edited.

mhvk

Nice! I like also that now we will have a good place to discuss other environment variables as well (like for building the system libraries). Would agree to keep this PR to just mostly moving the text around.

p.s. I do think it was helpful to see these changes on their own.

eerovaher · 2025-03-26T18:55:21Z

docs/changes/17932.other.rst

We don't announce documentation changes in the change log.

docs/config/index.rst

docs/development/testguide.rst

pllim · 2025-04-03T18:15:16Z

Thanks for this PR and the other one. Also the one Eero opened about testing doc. They are on my plate but I probably cannot get to them until next week. Thanks for your patience!

pllim

Thanks! It is good to finally have a dedicated section for this.

As for the links and contents, they were written a long time ago. The original motivation is now lost of buried in some really old commit/PR convo. I agree that updating them is out of scope here.

neutrinoceros added this to the v7.1.0 milestone Mar 25, 2025

neutrinoceros added the Docs label Mar 25, 2025

neutrinoceros assigned eerovaher Mar 25, 2025

neutrinoceros force-pushed the doc/env_vars branch from 4a36621 to 56abb43 Compare March 25, 2025 18:26

github-actions bot added config utils labels Mar 25, 2025

eerovaher reviewed Mar 25, 2025

View reviewed changes

neutrinoceros mentioned this pull request Mar 25, 2025

ENH: add support for setting astropy's cache and config directories via environment variables (blocked by #17934) #17640

Draft

1 task

eerovaher reviewed Mar 25, 2025

View reviewed changes

neutrinoceros force-pushed the doc/env_vars branch from 56abb43 to 54c95d7 Compare March 26, 2025 11:10

neutrinoceros marked this pull request as ready for review March 26, 2025 11:10

mhvk approved these changes Mar 26, 2025

View reviewed changes

eerovaher reviewed Mar 26, 2025

View reviewed changes

DOC: document runtime environment variables in a dedicated page

f44b36c

neutrinoceros force-pushed the doc/env_vars branch from f10b34c to f44b36c Compare April 4, 2025 12:37

neutrinoceros added the no-changelog-entry-needed label Apr 4, 2025

pllim approved these changes Apr 14, 2025

View reviewed changes

pllim merged commit 0318e2e into astropy:main Apr 14, 2025
32 of 33 checks passed

neutrinoceros deleted the doc/env_vars branch April 15, 2025 08:12

Uh oh!

Conversation

neutrinoceros commented Mar 25, 2025

Description

Uh oh!

github-actions bot commented Mar 25, 2025

Uh oh!

github-actions bot commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mhvk left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pllim commented Apr 3, 2025

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mhvk left a comment •

edited

Loading