API: make help methods raise an error if called under Python's optimized mode by neutrinoceros · Pull Request #17571 · astropy/astropy

neutrinoceros · 2024-12-20T10:57:26Z

Description

ref #17472
following the discussion in #17555
Although I marked this as an "API change" (because it technically is), I'd like to stress that this setup (PYTHONOPTIMIZE=2) was never explicitly supported and is very broken under astropy 7.0.x so it shouldn't actually be breaking to anyone.

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 · 2024-12-20T10:57:52Z

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 · 2024-12-20T10:57:54Z

👋 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 · 2024-12-20T18:00:15Z

I tried

$ python3 -OO
Python 3.12.3 (main, Nov  6 2024, 18:32:19) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from astropy.table import conf
>>> conf.help()

and

>>> conf.help("replace_warnings")

When calling help() without arguments the very first line was not quite what I expected because it did not mention table at all, but the rest of the output was fine. I don't see a good reason why we should bother with raising a RuntimeError. It might be a good idea to investigate why doesn't the fallback code at

astropy/astropy/config/configuration.py

Lines 102 to 104 in ce37ada

    
           else: 
        
               current_module = str(find_current_module(2)).split("'")[1] 
        
               header = f"Configuration parameters for `{current_module}`\n\n"

seem to work properly even though it is tested in

astropy/astropy/config/tests/test_configs.py

Line 252 in ce37ada

def test_configitem():

but if it turns out to be too complicated then perhaps we should just use "" as the fallback option.

Have you checked how the IO help methods behave with python -OO?

neutrinoceros · 2024-12-21T13:38:33Z

When calling help() without arguments the very first line was not quite what I expected because it did not mention table at all

Fair enough. I'll add self.__class__.__module__ to the error message.

if it turns out to be too complicated

it kinda does; the problem is that we have many tests for this function which check for the presence of specific fragments in the help messages, while the expected source of these fragment is unspecified and sometimes unclear. The fallback string doesn't have all these fragments, and neither would a simpler fallback.

About why I went with raising an exception; this approach has the benefit of being easily testable, while a non-blocking no-op (for instance) is in principle much harder to test; how can one check that there are zero unintended consequences to letting the function flow ?

neutrinoceros · 2024-12-21T13:58:03Z

Fair enough. I'll add self.__class__.__module__ to the error message.

Actually, I'm not sure this is a good idea, because the help function is normally exposed as attached to a class instance whose name cannot be inspected when creating the error message. For instance, there's no way for me to get the following behavior

>>> from astropy import conf
>>> conf.help()
RuntimeError: astropy.conf.help is not available

without hard-coding conf into the error message, which seems like it could easily create more confusion than it'd save, if a configuration object was to be exposed under a different name, which could maybe happen downstream ?

github-actions · 2025-05-20T05:44:04Z

Hi humans 👋 - this pull request hasn't had any new commits for approximately 4 months. I plan to close this in 30 days if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock.

If you believe I commented on this pull request incorrectly, please report this here.

nstarman · 2025-05-20T13:20:26Z

          module=astropy
          value=False
        """
+        if sys.flags.optimize >= 2:


Are we planning to have a test for Python in optimized mode?

one eternity later: yes (#17472)

nstarman · 2025-05-20T13:21:55Z


 OLD_CONFIG = {}

+SKIPIF_OPTIMIZED_PYTHON = pytest.mark.skipif(


Can this be moved to a common conftest? I see this is copied in another file.

It's a bit cumbersome, but this got me thinking there's a better way: subprocesses. I'll give it a try now.

turns out if was actually much easier to define pytest markers

nstarman · 2025-05-20T13:23:20Z

        out : None or file-like
            Output destination (default is stdout via a pager)
        """
+        if sys.flags.optimize >= 2:


What about making a PYTHON_OPTIMIZED in astropy.utils ?

I'm not seeing the benefit, I prefer keeping this as is if you don't mind

nstarman

largely LGTM. Some code cleanup requested. I think it's generally good to have informative errors, and this simple check in the help methods looks to improve the error message when calling help in optimized mode.

…zed mode

neutrinoceros · 2026-04-27T11:10:08Z

Sorry for this record-breaking wait (>1year !). I think I've addressed (or replied to) all your comments.

neutrinoceros added this to the v7.1.0 milestone Dec 20, 2024

neutrinoceros added the API change PRs and issues that change an existing API, possibly requiring a deprecation period label Dec 20, 2024

github-actions Bot added config io.ascii io.registry labels Dec 20, 2024

neutrinoceros force-pushed the config/io.registry/api/raise_from_help_in_optimized_mode branch from 6a767ad to 0abc647 Compare December 20, 2024 10:59

neutrinoceros mentioned this pull request Dec 20, 2024

TST: add a CI job for Python's optimized mode #17472

Draft

1 task

neutrinoceros force-pushed the config/io.registry/api/raise_from_help_in_optimized_mode branch from 0abc647 to b63c7a7 Compare December 20, 2024 11:20

neutrinoceros marked this pull request as ready for review December 20, 2024 11:51

neutrinoceros requested review from dhomeier, nstarman and taldcroft as code owners December 20, 2024 11:51

neutrinoceros mentioned this pull request Dec 21, 2024

API: make delay_doc_updates context manager a noop under Python's optimized mode #17572

Merged

1 task

astrofrog modified the milestones: v7.1.0, v7.2.0 Apr 28, 2025

github-actions Bot added the Close? Tell stale bot that this issue/PR is stale label May 20, 2025

neutrinoceros added keep-open and removed Close? Tell stale bot that this issue/PR is stale labels May 20, 2025

nstarman reviewed May 20, 2025

View reviewed changes

astrofrog modified the milestones: v7.2.0, v8.0.0 Nov 3, 2025

neutrinoceros force-pushed the config/io.registry/api/raise_from_help_in_optimized_mode branch from b63c7a7 to cd2dc48 Compare April 27, 2026 10:26

neutrinoceros removed keep-open labels Apr 27, 2026

neutrinoceros force-pushed the config/io.registry/api/raise_from_help_in_optimized_mode branch from cd2dc48 to a937edf Compare April 27, 2026 10:28

API: make help methods raise an error if called under Python's optimi…

fdf135f

…zed mode

neutrinoceros force-pushed the config/io.registry/api/raise_from_help_in_optimized_mode branch from a937edf to fdf135f Compare April 27, 2026 11:07

nstarman approved these changes Apr 27, 2026

View reviewed changes

neutrinoceros merged commit 99ce0c3 into astropy:main Apr 27, 2026
36 of 38 checks passed

neutrinoceros deleted the config/io.registry/api/raise_from_help_in_optimized_mode branch April 27, 2026 15:06


		OLD_CONFIG = {}

		SKIPIF_OPTIMIZED_PYTHON = pytest.mark.skipif(

Conversation

neutrinoceros commented Dec 20, 2024

Description

Uh oh!

github-actions Bot commented Dec 20, 2024

Uh oh!

github-actions Bot commented Dec 20, 2024

Uh oh!

eerovaher commented Dec 20, 2024

Uh oh!

neutrinoceros commented Dec 21, 2024

Uh oh!

neutrinoceros commented Dec 21, 2024

Uh oh!

github-actions Bot commented May 20, 2025

Uh oh!

nstarman May 20, 2025

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

nstarman May 20, 2025

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

nstarman May 20, 2025

Choose a reason for hiding this comment

Uh oh!

neutrinoceros Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

nstarman 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!

neutrinoceros commented Apr 27, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nstarman left a comment •

edited

Loading