-
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Update pyupgrade config for Python 3.9 #9987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| # NOTE: The guard for Python 3.9 is because types.GenericAlias is only added in Python 3.9. This is not a problem | ||
| # as the syntax is added in the same version in the first place. | ||
| if (3, 9) <= sys.version_info < (3, 11) and isinstance(annotation, types.GenericAlias): | ||
| if sys.version_info < (3, 11) and isinstance(annotation, types.GenericAlias): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be 3, 10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@auvipy Can you please give more details? This upper version limit is not something I touch with this PR and I don't know much about why it's implemented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears only this was removed:
# NOTE: The guard for Python 3.9 is because types.GenericAlias is only added in Python 3.9. This is not a problem
# as the syntax is added in the same version in the first place.
if (3, 9) <=Which matches the scope of the PR. Anything we’re missing @auvipy ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am considering whether we need to revert this specific change @ulgens .
Original code:
def annotation_is_class(annotation: typing.Any) -> bool:
"""Test if a given annotation is a class that can be used in isinstance()/issubclass()."""
# isclass() returns True for generic type hints (e.g. `list[str]`) until Python 3.10.
# NOTE: The guard for Python 3.9 is because types.GenericAlias is only added in Python 3.9. This is not a problem
# as the syntax is added in the same version in the first place.
if (3, 9) <= sys.version_info < (3, 11) and isinstance(annotation, types.GenericAlias):This is mostly a safeguard, a functionality, that we might actually want to keep for a few more years until Python 3.9 is really out of the game, or, if we officially drop existing support for edge cases, which I prefer not to, at this moment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Nusnus The change doesn't affect the use of 3.9, it's for anything earlier than 3.9. Because 3.9 is the earliest supported version, (3, 9) <= sys.version _info will always be true - it doesn't need to be checked.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9987 +/- ##
=======================================
Coverage 78.68% 78.68%
=======================================
Files 153 153
Lines 19330 19337 +7
Branches 2221 2220 -1
=======================================
+ Hits 15209 15215 +6
Misses 3824 3824
- Partials 297 298 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Nusnus
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this PR!
LGTM so far, though we’ll need to wait before deprecating 3.9 in main.
|
@Nusnus Thank you. To clarify, this PR doesn't serve to the deprecation of 3.9 - it cleans residues from <3.9 support and can be merged safely in the current release. |
These were added in * celery@f1ddd58 * celery@c4e4bab but they are not helpful. Decided to clean in celery#9987 (comment)
These were added in * f1ddd58 * c4e4bab but they are not helpful. Decided to clean in #9987 (comment)
9fe86c9 to
afb57e4
Compare
It seems this wasn't handled during 3.8 removal
afb57e4 to
06ab87f
Compare
|
@Nusnus Is there any chance to handle this in 5.6.0? |
Description
Update pyupgrade config for Python 3.9. It seems this wasn't handled during 3.8 removal.