-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
Add __orig_bases__ to TypedDict #150
Conversation
This should be up to date with the CPython change which just got merged |
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.
Thanks! Could you also add a CHANGELOG entry? Look at some of Alex's recent PRs for examples.
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.
We need to change these lines so that we reimplement NamedTuple on <=3.11 rather than <=3.10. Otherwise call-based typing_extensions.NamedTuple
s will have __orig_bases__
on 3.7-3.10 and 3.12+, but not 3.11 😄
typing_extensions/src/typing_extensions.py
Lines 2316 to 2320 in fb37b2e
# Backport typing.NamedTuple as it exists in Python 3.11. | |
# In 3.11, the ability to define generic `NamedTuple`s was supported. | |
# This was explicitly disallowed in 3.9-3.10, and only half-worked in <=3.8. | |
if sys.version_info >= (3, 11): | |
NamedTuple = typing.NamedTuple |
test_typing_extensions_defers_when_possible
will need to be adjusted as a result -- you'll need to move "NamedTuple"
into the if sys.version_info < (3, 12)
set here:
typing_extensions/src/test_typing_extensions.py
Lines 3610 to 3613 in fb37b2e
if sys.version_info < (3, 11): | |
exclude |= {'final', 'NamedTuple', 'Any'} | |
if sys.version_info < (3, 12): | |
exclude |= {'Protocol', 'runtime_checkable', 'SupportsIndex'} |
Done, I copied Alex's changelog note from CPython. |
@@ -813,6 +813,15 @@ def _typeddict_new(*args, total=True, **kwargs): | |||
raise TypeError("TypedDict takes either a dict or keyword arguments," | |||
" but not both") | |||
|
|||
if kwargs: | |||
warnings.warn( |
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 think it's important that we copy CPython's behaviour here, and CPython emits a deprecation warning here on 3.11+. This PR means we reimplement TypedDict on 3.11 now, so the change is needed, I think. @JelleZijlstra do you think we should only emit the deprecation warning if the user is running typing_extensions on 3.11+? (Referencing #150 (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.
That's a hard question and gets into what we would do if we ever want to break backwards compatibility for typing-extensions. My first instinct is to say we should always warn in typing-extensions, regardless of the version, but then what would we do when CPython removes support for kwargs-based TypedDicts? I'd be hesitant to remove the runtime behavior in typing-extensions and break backwards compatibility.
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 agree it's a hard question.
Elsewhere in the typing_extensions
implementation of TypedDict
, we actually already have two places where CPython long ago removed support for a certain feature, but typing_extensions
has in effect had "eternal deprecation warnings":
typing_extensions/src/typing_extensions.py
Lines 789 to 791 in fb37b2e
import warnings | |
warnings.warn("Passing '_typename' as keyword argument is deprecated", | |
DeprecationWarning, stacklevel=2) |
typing_extensions/src/typing_extensions.py
Lines 804 to 806 in fb37b2e
import warnings | |
warnings.warn("Passing '_fields' as keyword argument is deprecated", | |
DeprecationWarning, stacklevel=2) |
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 think eternal deprecation warnings might be the right answer, actually. (Or eternal until we ever make typing-extensions 5 in the distant uncertain future.)
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.
So maybe we do want the change in #150 (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.
Yes, let's do that. I commented there because the change looked unrelated to this PR.
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
I filed adriangb#2 as a PR to your branch to address my comment above here: #150 (review) |
Don't skip NamedTuple tests on 3.11+. Also make them pass on 3.11+.
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.
Looks great to me!
Co-authored-by: AlexWaygood <alex.waygood@gmail.com>
No description provided.