-
-
Notifications
You must be signed in to change notification settings - Fork 125
Fix how type parameters are collected from parametrized Protocol bases
#667
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?
Fix how type parameters are collected from parametrized Protocol bases
#667
Conversation
| typevar_types = (TypeVar, typing.TypeVar, ParamSpec) | ||
| if hasattr(typing, "ParamSpec"): # Python 3.10+ | ||
| typevar_types += (typing.ParamSpec,) | ||
| tvars = _collect_type_vars(cls.__orig_bases__, typevar_types) |
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.
note: for compatibility with both 3.9 and 3.10+, c.f. python/cpython#26091
| args, | ||
| *, | ||
| enforce_default_ordering=_marker, | ||
| validate_all=False, |
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.
note: since typing only explicitly passes validate_all from inside _generic_init_subclass (which this monkey patches below), there's no need for sys._getframe hacks like what was done for enforce_default_ordering in #392.
| if enforce_default_ordering is _marker: | ||
| enforce_default_ordering = _has_generic_or_protocol_as_origin() |
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.
note: using a placeholder default value for enforce_default_ordering allows the same function to work as _collect_parameters on Python <3.13 and as _collect_type_parameters on Python 3.13+. Another alternative would be to split this into separate <3.13 and 3.13+ implementations in respective version branches
| def _collect_parameters( | ||
| args, | ||
| *, | ||
| enforce_default_ordering=_marker, | ||
| validate_all=False, | ||
| ): |
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 saw that there have been previous issues where monkey patching typing internals can break future versions of typing if new parameters are added (c.f. python/cpython#118900). I think there's low risk of that here, since this only monkey patches internal functions on Python <3.15, and it seems unlikely that new parameter would be backported to 3.14 at this point. But if we wanted to help guard against that, I suppose adding an ignored **kwargs parameter might be an option?
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #667 +/- ##
==========================================
- Coverage 97.36% 97.30% -0.07%
==========================================
Files 3 3
Lines 7680 7768 +88
==========================================
+ Hits 7478 7559 +81
- Misses 202 209 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Fixes #636, backport of python/cpython#137281
Added some inline comments about the implementation below.