-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
PEP 728: Address feedback from Carl #4393
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
Conversation
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.
Thank you! I was planning to do this, but clearly hadn't gotten to it yet. This update looks great to me, and addresses all of my comments. Just one comment inline on something that still doesn't quite look right to me.
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.
LGTM (assuming the review comments are addressed)
Thanks for the review! Pushed an update. Left a conversation open to confirm if the edit makes sense. |
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.
This looks good to me. We've had a couple days for reviews, I think I will go ahead and merge.
Discussion:
https://discuss.python.org/t/pep-728-typeddict-with-typed-extra-items/45443/153
This mainly clarifies indexed accesses and assignments with arbitrary str keys.
cc @carljm :
Let me know if there is anything else you have in mind!
PEP 123: Summary of changes
)📚 Documentation preview 📚: https://pep-previews--4393.org.readthedocs.build/