If this was for python < 3.10, can we not just remove this import?

I wasn't sure at first, since this import has been propagating in the code base for seemingly different reasons since this comment was written, but upon checking I think you're right. Thanks !

For here probably fine, but let me ping @nstarman to ask if we still need from __future__ import annotations in general now that our minimum python is 3.10.

Can't we just remove the private function entirely?

done

The intention behind having these PYTHON_LT_* variables was that when we drop support for some Python version then we can simply delete the definition of the corresponding variable causing all the code that is using it to raise an error, and this would prevent compatibility code from staying around longer than it is needed. But this setup also means that strings like that should never be grepped for, so I'm not convinced this change is helpful.

I also think we should stop using this PYTHON_LT_* variables because we are now using Ruff and its rule UP036 can recognize comparisons involving sys.version_info calls and update the code automatically (possibly requiring the --unsafe-fixes option).

The best thing to do here would be to replace the comment with code that explicitly uses sys.version_info.

For the numpy equivalent, I tend to do the find astropy -name \*.py -exec grep NUMPY_LT... - but if it is easy to refactor with a version check, that would indeed be ideal.

Amusingly, I wrote UP036 in the original (pyupgrade) code, so I'm well aware it exists 😅
However my goal here is absolutely not to revolutionise how astropy code is written, and I believe it would be best left out of this PR ?

@mhvk wrote

For the numpy equivalent, I tend to do the find astropy -name \*.py -exec grep NUMPY_LT...

git grep NUMPY_LT_... is shorter, produces more useful output (unless you add something to that find incantation of yours that you didn't write down) and faster even if you don't specify you are only interested in .py files in astropy/ (although you could with git grep NUMPY_LT_... -- 'astropy/*.py').

@neutrinoceros wrote

However my goal here is absolutely not to revolutionise how astropy code is written, and I believe it would be best left out of this PR ?

The title of your pull request says that it is removing Python 3.9 compatibility code, so this particular change would be out of scope here. But overall it would be good to write (or rewrite) any compatibility code in a way that prevents it from outliving its usefulness.

Thanks, learn something new every day!

Initially I misread the comment and thought it was not relevant in 3.10. Happy to just leave it out of this PR.

But overall it would be good to write (or rewrite) any compatibility code in a way that prevents it from outliving its usefulness.

for the record I agree (this is what I wrote UP036 for), but I think I recall there was some objections when I proposed this refactor in astropy some time ago (but I can't remember the exact context).

(I dropped the change)

FWIW, what I remember about auto-upgrade is mostly that some of us (including me) felt that one should not upgrade code just for the sake of it (i.e., one doesn't have to use |= to update a dict, using .update() is fine).

Automatically recognizing pieces of code that are no longer useful does not fall in that category, though!

I'll open another PR to propose this.

for posterity: this is #16186

If from __future__ import annotations was only needed to enable writing down union types using the |-operator then the import can be safely removed now. However, in files that have an if TYPE_CHECKING: block removing these lines would require rewriting (possibly many) annotations as string literals, which doesn't sound helpful. Luckily it is simple to find out if this import is needed or not: if it is then removing it causes the code to fail already at import time.