GeezerVison is showing github.com — gh-137840: Implement PEP 728 (closed and extra_items in typing.TypedDict) by angela-tarantula · Pull Request #137933 · python/cpython
gh-137840: Implement PEP 728 (closed and extra_items in typing.TypedDict)
#137933
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.
There was an error while loading. Please reload this page.
Copy link
Copy Markdown
ContributorAuthor
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 looks a bit odd as an arbitrary extra_items value, but it's valid, since it just means extra keys must have values assignable to range. For context, this test is copied from typing_extensions, where it was used to cover the same case. We could edit it for readability, but I kind of like it as a reminder for maintainers that extra_items accepts any type, not just the obvious ones.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The reason will be displayed to describe this comment to others. Learn more.
It's still valid/correct as a negative test. By "valid" do you mean "correct" or "readable?" Passing range into extra_items is valid, and passing closed=True at the same time should raise a runtime error.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
That would make a solid test, both understandable and useful.
Meanwhile, the negative test, class TD(TypedDict, closed=True, extra_items=range): --> error would be better served with a simpler, more straightforward extra_items=int argument.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Copy link
Copy Markdown
ContributorAuthor
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 for clarifying. I thought about it some more.
I don't think introducing an extra_items=range counterpart would actually widen the test coverage, since it wouldn't be exercising any new behavior. PEP 728 splits responsibilities between runtime and type checker behavior. While extra_items is only supposed to accept a valid type expression, validating that is the type checker's job (e.g. MyPy's valid-type error), not the runtime's. The runtime just stores whatever is passed in.
So the real subject under test is simply:
Child.__extra_items__ must correctly store the value passed into extra_items.
We don't need multiple values to prove that behavior.
And although range is a less obvious type, I think it makes sense in the negative test. That test is specifically asserting the error message “Cannot combine closed=True and extra_items”. Using range highlights that the failure comes from the combination, not from range itself being an invalid value of extra_items.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
The reason will be displayed to describe this comment to others. Learn more.
I tend to use range sometimes as it's a builtin type that isn't generic (like list) and doesn't participate in promotion weirdness (like float and historically bytes), so it's a good basic type to test with.
Plus, people who forget that range is a type get to learn something :)
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
brianschubert, AlexWaygood, and angela-tarantula reacted with heart emoji
All reactions
❤️3 reactions
x: str
def test_annotations(self):
# _type_check is applied
with self.assertRaisesRegex(TypeError, "Plain typing.Final is not valid as type argument"):
:class:`typing.TypedDict` now supports the ``closed`` and ``extra_items``
keyword arguments (as described in :pep:`728`) to control whether additional
non-required keys are allowed and to specify their value type.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up. Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.