-
-
Notifications
You must be signed in to change notification settings - Fork 110
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
Initial support for TypeVarLike default parameter (PEP 696) #77
Conversation
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.
Not entirely sure how I feel about these subclassing the typing versions if possible but apart from the one comment LGTM
Co-authored-by: James Hilton-Balfe <[email protected]>
I agree. When I first experimented with it, it felt strange. However, with subclassing we don't need to worry about the base implementation and avoid reimplementing that again. |
I hadn't considered that, although it would still probably be a good idea to sys.version_info checks to make sure that you aren't redefining the behaviour if TypeVarLikes already have the default argument |
@@ -1147,6 +1148,42 @@ def __repr__(self): | |||
above.""") | |||
|
|||
|
|||
class _DefaultMixin: |
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.
This class should define empty slots to make TypeVar not have dict
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.
Done. Although I'm not sure it will work. For 3.10 _TypeVarLike
and for 3.11 _BoundVarianceMixin
don't add __slots__
, so the __dict__
for TypeVar
is created anyway.
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.
If you're interested, we can fix that for 3.12. (I'd be hesitant for older versions since it may break some users.)
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.
Seems like __slots__
was removed on purpose since it didn't have any use. I've updated the PR to remove it from TypeVar
as well. Ref: python/cpython#30444
Co-authored-by: Alex Waygood <[email protected]>
Just realised that the default for the default argument shouldn't be None and this needs a new object that should be the default as you currently can't make a distinction between default=None and default=empty |
Should be easy enough to implement. Just wondering now why |
I expect this just wasn't considered, and it's never come up as a problem because a bound of (Then again, pyright just got a bug report asking for |
Closes #75
Ref: https://peps.python.org/pep-0696/