| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| :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. |
| Back | FazBrowse Home | New Git URL |
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIs range the builtin here?
That's a bit strange.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI 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.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThat could be valid for a regular test that validates stored values.
this however is a negative test, isn’t it?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityIt'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.
Uh oh!
There was an error while loading. Please reload this page.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualitySorry I was being terse.
I'm trying to say that this test:
Could have an extra_items=range counterpart.
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 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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityP.S. my comment overall is minor, please don't let me stop your work!
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityThanks 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:
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.
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.
Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low QualityI 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.