-
Notifications
You must be signed in to change notification settings - Fork 248
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 more information to resolved TypedDicts #654
add more information to resolved TypedDicts #654
Conversation
Codecov Report
@@ Coverage Diff @@
## master #654 +/- ##
==========================================
- Coverage 98.80% 98.40% -0.40%
==========================================
Files 63 63
Lines 7005 6767 -238
==========================================
- Hits 6921 6659 -262
- Misses 84 108 +24
Continue to review full report at Codecov.
|
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.
Hey @JoshFerge, thanks for the PR. excellent stuff!
the breakage is fine i think because it's the correct thing to do. i suppose it would have broken anyway if someone would have used the constructor init with missing arguments (TD1(**data)
), right?
LGTM with the doc retrieval change
Yep, exactly |
|
||
required = None | ||
|
||
if sys.version_info >= (3, 9): |
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'm going to do some quick testing sometime this week to see if it just works with the typing_extension TypedDict as well (what we're using since we're not on 3.9 yet), in which case we can simply check the hint for the __required_keys__
attribute.
Hey, I fixed the call and found a issue with this new case in |
Yup! Thanks! |
Also use feature detection for __required_keys__ instead of probing Py version
__required_keys__
field to annotate TypedDict values with required keys__doc__
attribute if it existsThis would be a breaking change resulting in different OpenAPI schema if end-users didn't mark their TypedDicts with
total=False
on 3.9 and above, as previously, all TypedDict fields were optional.