BUG: Ensure import astropy.coordinates works with -OO optimize flag - #15037

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 "When to rebase and squash commits".
• 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?
• Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
• 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.

mpldev failure is unrelated

I thought about maybe adding a job to test for -OO imports but is that a good idea? Where? Maybe @astropy/astropy-project-release-team and @astropy/devops-maintainers have thoughts.

@eerovaher , are you saying to only run the affected code for sys.flags.optimize == 0 only?

Yes, except that the -O flag still handles docstring normally, so the actual condition to check would be sys.flags.optimize < 2.

But what does "Currently Nuitka sets optimize=1 for flag -OO" mean?

But what does "Currently Nuitka sets optimize=1 for flag -OO" mean?

Nuitka is 3rd party software. Last week it incorrectly set optimize=1 when python flag is -OO. This problem was fixed in the factory release (not on pip yet) of Nuitka. I thought the problem I'm seeing in my code that uses both Nuitka and Astropy was caused by Astropy not handling the python -OO flag, but now I think there are 3 separate (but maybe related) issues: Astropy -OO problem (this PR), Nuitka optimize flag incorrect setting (fixed in Nuitka), and an unresolved issue.

an unresolved issue

Interesting, so those two patches together still did not fix everything? What else?

Interesting, so those two patches together still did not fix everything? What else?

I'm really not sure. My plan was to try to update ply and see if that helped, but I'm not getting anywhere with that due to my limited python skills. My next idea is to wait until this PR and Nuitka's fix are in the mainline versions of both code bases and try again with clean installs of each. (I don't have high hopes this will fix the problem because my local versions show improvement for simplified tests of each bug, so I think I applied the fixes correctly.)

Given the conversation thus far, I am not convinced that strict optimize flag check is any nicer than the existing one, as it seems like cls.__doc__ maybe be None for slight different scenario. Since there are already 2 approvals, let's get this in so we can smoke out the third problem, but @eerovaher, feel free to submit follow-up PR if you feel strongly about using optimize flag.

I will also open a follow up issue about testing. See #15052

Thank you, all!

io.fits (and other modules) already checks if __doc__ is None (or a string), since #3923. This seems the best option to me, compared to checking a specific value of sys.flags.optimize.

I suggested explicitly checking sys.flags because I thought that would make the code more self-documenting so that there would be no need to add comments, but neither of the updated Python files imports sys, and I can agree that importing it just to avoid writing two simple comments is excessive.