BUG: ensure that docstrings for read/write public methods are always available for inspection (nddata) - #17555

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.

👋 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?

The errors here are puzzling. I'll come back to this later today if I have time.

Actually the problem was quite trivial, I'm just paying the price of context switching.

tl;dr -- Didn't we decide in another PR that docstring missing in optimized mode is okay? So why this special treatment here?

The help function requires the docstring. We could either: have a broken function, refactor the help function to be a no-op in optimized mode, or keep the docstring.

refactor the help function to be a no-op

That would be simpler, right, and localize this optimized mode stuff in one file instead of spread it out over different subpackages that hook into the unified I/O?

I suppose the Q though is: when do people use python's optimized mode? If it's only to do production runs, then making .help() a no-op is fine. If it's also to use Astropy in an IDE, then...

In Python the natural way of documenting things is using docstrings. The fact that a few help() methods rely on docstrings and break if they are not available is therefore not surprising. If you want to be helpful for users that do manage to stumble upon this problem then the sensible thing to do would be to raise a RuntimeError in the help() with an informative message.

Independently from the previous point, if a user has explicitly told Python to remove the docstrings then any clever tricks that insert the docstrings anyways clearly go against the user's wishes.

Yes, making help functions either no-ops or runtime errors in optimized mode would actually be more sensible and easier to implement/maintain. I'm slightly leaning towards "noop" but I don't have strong opinion. Would that work for you guys ?

👍 to not sneakily trying to keep docstrings, and instead adjusting help. I'd lean to not adjusting help at all and just letting it fail, but am OK with a more informative RuntimeError too. I don't like a no-op.

Someone using PYTHONOPTIMIZE=2 should not expect things like .help() to work. This is not a normal mode for e.g. interactive work (the only benefit to gain a few kb by removing docstrings), so can we avoid adding tricks and made the code more complex than needed to support invalid usecases ? (Same for config #17548 and maybe others?)
.help() can just return None or even raise an exception in that case (and we should not care about it and waste time on this, there is really no reason to use this mode unless you need to fit a ton of code on very limited hardware).

Yes, in hindsight this seems like the correct approach. I just didn't know how much tinkering I'd need before I gave it a try, but I agree that it's too much. Let's just close this, as well as #17557 and revert #17548.
I'll open a new PR tomorrow with the more consensual and simple approach.
Thanks all !

Thanks, @neutrinoceros -- Sometimes one needs to see the code to realize it is the wrong approach!