RFC: inline usage of sys.version_info to enable automated clean up of compatibility code#16186
Conversation
|
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.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
neutrinoceros
force-pushed
the
rfc/auto_upgradable_compat_code
branch
from
df1e6dc to
21b377a
Compare
neutrinoceros
force-pushed
the
rfc/auto_upgradable_compat_code
branch
from
21b377a to
d286c11
Compare
| if PYTHON_LT_3_11 | ||
| else f"property 'name' of {cosmo.__class__.__name__!r} object has no setter" | ||
| ) | ||
| if sys.version_info < (3, 11): |
There was a problem hiding this comment.
note to reviewers: the reason I also refactored the ternary out is that, to the best of my knowledge, UP036 auto cleanup only works for if/else patterns. That is, unless the ruff reimplementation was made smarter than my original code.
… compatibility code
neutrinoceros
force-pushed
the
rfc/auto_upgradable_compat_code
branch
from
d286c11 to
92b843c
Compare
| else: | ||
| mark = lambda f: f | ||
|
|
||
| @mark |
There was a problem hiding this comment.
Instead of introducing no-op marker, maybe cleaner to use warnings.filterwarnings context manager inside this function?
There was a problem hiding this comment.
It would, only I don't know which part of this long test is supposed to trigger that warning (and I can't seem to reproduce it locally), so it seemed to me that a no-op decorator would at least keep the diff minimal.
There was a problem hiding this comment.
I was gonna suggest the whole test go inside the context manager but yeah, would be larger diff. Leave it as is then. Thanks!
mhvk
left a comment
mhvk
left a comment
There was a problem hiding this comment.
I quite like getting rid of the PYTHON_LT_* stuff. Thanks!
nstarman
left a comment
nstarman
left a comment
There was a problem hiding this comment.
Everything LGTM! Very nice to not have to remember to upgrade this code in the future.
|
Thanks, all! |
4 participants
Description
Following the discussion in #16183 (comment)
This one is on the house (I'm not going to bill it), I'm just mostly scratching my own itch
The goal is to make future PRs similar to #15063 a tad easier to write and maintain by enabling automatic cleanups for compatibility branches using the
UP036rule from pyupgrade (and re-implemented in ruff).I also note that there's actually an existing precedent for using
sys.version_infoinstead ofPYTHON_LT_*internally, so this actually uniformizes the code base.I believe @eerovaher @mhvk and @nstarman would be interested in this.