Skip to content
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

Merged
merged 6 commits into from Oct 3, 2022
Merged

Initial support for TypeVarLike default parameter (PEP 696) #77

merged 6 commits into from Oct 3, 2022

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Oct 2, 2022

Copy link
Contributor

@Gobot1234 Gobot1234 left a 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

src/typing_extensions.py Outdated Show resolved Hide resolved
Co-authored-by: James Hilton-Balfe <gobot1234yt@gmail.com>
@cdce8p
Copy link
Contributor Author

cdce8p commented Oct 2, 2022

Not entirely sure how I feel about these subclassing the typing versions if possible

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.

@Gobot1234
Copy link
Contributor

Not entirely sure how I feel about these subclassing the typing versions if possible

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:
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Member

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.)

Copy link
Contributor Author

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 <Alex.Waygood@Gmail.com>
@JelleZijlstra JelleZijlstra merged commit fafb5c5 into python:main Oct 3, 2022
@cdce8p cdce8p deleted the TypeVarLike-defaults branch October 4, 2022 01:36
@Gobot1234
Copy link
Contributor

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

@cdce8p
Copy link
Contributor Author

cdce8p commented Oct 7, 2022

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 None was chosen as default for __bound__. True, a TypeVar with bound=None doesn't make much sense but that can't be the only reason, is it?
https://github.com/python/cpython/blob/v3.11.0rc2/Lib/typing.py#L940-L943

@JelleZijlstra
Copy link
Member

I expect this just wasn't considered, and it's never come up as a problem because a bound of None has no practical use.

(Then again, pyright just got a bug report asking for NewType(None) to work, so maybe someone has a use for subtypes of None.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Initial runtime support for PEP 696 (TypeVarLike defaults)
4 participants