DEP: bump minimal supported IPython version to 7.32, declare ipykernel and ipywidgets as optional deps, cleanup

[neutrinoceros]

Description

In the same vein as #16353, but specifically for IPython since this one requires a "bump" (effectively I'm just declaring as minimal the first version that supports Python 3.10, which is still a little bit shy of SPEC 0's current recommendation at 7.39).

• 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]

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]

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

[neutrinoceros]

IPython.zmq was removed in 2013 in ipython/ipython#4121 and this functionality was moved to IPython.kernel, which was itself deprecated in IPython 4.0.0 and removed in 8.0.0, see ipython/ipython#13386

[pllim]

I don't know what this NameError was for but looks like handling it has been dropped in this PR. Can you please clarify? Thanks!

[neutrinoceros]

The get_ipython method was oddly written in such a way that its return variable was only bound if IPython was installed, meaning it was effectively relying on a NameError being raised otherwise. Since I also rewrote the method in a more conventional manner, this handling isn't needed any more

[pllim]

Still, looking at code in main, the original design looks like it never intended to throw exception if IPython is not installed, but this PR does. Can we refactor in a way that does not throw exception? Thanks for your patience!

[neutrinoceros]

I'm confused

original design looks like it never intended to throw exception if IPython is not installed

if you mean the get_ipython class method, it seems to me that it was actually designed to throw an exception since that's how it's been used through except NameError (albeit a quirky exception to catch in your own code).

but this PR does

I don't understand, where else would an exception be raised in my version ?

[pllim]

Actually the NameError on main makes no sense to me. When I try to emulate the implementation, I get UnboundLocalError. But that is neither here nor there...

The point I was trying to make is that get_ipython on main defers the exception handling to OutStream. But in this PR, you explicitly throws ModuleNotFoundError from within get_ipython if IPython is not installed. I think that is a subtle behavior change?

[neutrinoceros]

actually it was easy enough that I just went for it: #16369

[neutrinoceros]

Also, I figured that this _IPython.get_ipython method was not actually needed, so I refactored it. To make it easier for you to follow after I rebased, I've kept this one change as a separate commit (that should eventually get squashed).

[pllim]

Thanks, does look much cleaner now. But I still don't know what is the point of this whole _IPython class now that I dig into the code. I can't seem to see it being used anywhere and it is "hidden".

Anyways, should we get #16369 in first?

[pllim]

Ah nvm, found it. Just used internally here for isatty.

[neutrinoceros]

should we get #16369 in first?

these PRs are actually completely independent as it turns out; they can be merged in any order

[pllim]

Thanks. LGTM now!