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.
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
PEP 764: Inlined typed dictionaries #4082
base: main
Are you sure you want to change the base?
PEP 764: Inlined typed dictionaries #4082
Changes from 1 commit
c364510
ac5786b
c058d0e
76dc942
2e0c379
2857d69
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 don't think we should support nested inlined TypedDict definitions as shown above. We should require that the type annotation for each item be a valid type expression, and a dictionary expression not enclosed by a
TypedDict
is an invalid type expression. The above example should therefore be changed to:The grammar definition proposed above by Jelle already implies this, but the spec should make it clear that using nested dictionary expressions (like in the current code sample) is not allowed and should result in a type checker error.
I understand this might seem needlessly verbose, but if we allow dictionary expressions in type expressions outside of a
TyepdDict
type argument, it will create a cascade of exceptions and future composability issues. For example, we'd need to support raw dictionary expressions withinRequired
,NotRequired
, andReadOnly
.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.
Indeed using a nested inlined typed dictionary is more consistent! Changed.
(Using
typing.Dict
instead ofTypedDict
might help reducing verbosity here).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 that
Required
,NotRequired
andReadOnly
should be permitted here. Absent any mention of this, the typing spec could be interpreted as disallowing this. It's better to make it clear.I presume that all inlined TypedDicts are implicitly "total" (i.e. all items are assumed to be
Required
unless explicitlyNotRequired
). This makes sense because it's consistent with precedent, but it would be good to spell it out in the spec.I think it's also worth considering making all inlined TypedDicts implicitly "closed" (as defined in draft PEP 728). This would deviate from precedent, but I think one could make a good argument for why inlined TypedDicts should be closed.
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.
Updated.
Sounds reasonable. I have to say I'm still a bit confused with PEP 728 and
closed=True
. If we disallow subclassing inlined typed dictionaries, it means that every inlined typed dictionary is implicitly@final
. What's the purpose of havingclosed=True
then? I guess I should ask my question in the PEP 728 discussion thread instead.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.
Am I correct in assuming that there is no way for an inlined TypedDict to "extend" another (named) TypedDict? One could imagine supporting this using dictionary expansion syntax, but that adds complexity that may not be justifiable. It would also impose additional runtime requirements on TypedDict classes (they'd need to be iterable).
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.
A good way to support inheritance could be to allow more than one argument to the subscript:
TypedDict[Base, {"a": int}]
.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.
Pyright 1.1.387, which will be published in the next day, includes experimental support for the latest draft of this PEP.
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.
Amazing, updated to reference the future release.
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.
Did you consider reusing
typing.Dict
here? I can see some potential advantages. It's less verbose, doesn't have the baggage ofbuiltins.dict
, and the fact that this is a "typed" dict is already implicit given that it is imported fromtyping
and involves type annotations.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.
Showing my TypeScript bias here 😅 but I'm wondering how sacrilegious it would be to avoid the subscription all together and simply use the dictionary itself?
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 causes problems with the
|
operator at least. I've been thinking of writing up something like "Why can't we ...?" discussing why certain "obvious" ways to make typing more ergonomic may not work well in practice.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 that such writeup would be very useful!
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 added an extra section in the rejected ideas regarding the plain dict syntax.
On using
typing.Dict
, I kind of like the idea for the reasons you mentioned @erictraut.Dict[{...}]
implicitly spells out as something like:However, I still think this is debatable for a couple reasons:
typing.Dict
is currently marked as deprecated in the Python documentation (although not scheduled for removal). It might be confusing to undeprecate it.I'll add an entry to the open issues so that we can further discuss this option.
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.
No, this shouldn't be allowed by a static type checker. In this example,
InlinedTD
is a type alias.If you change this to
InlinedTD = TypedDict('InlinedTD', {'a': int})
, then it should be allowed (as it is today) becauseInlinedTD
is now a class, which can be used as a base class.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, I think we should also discuss whether an inlined typed dictonary is a proper class or an instance of some new internal
typing._InlinedTypedDict
class. As specified currently in this PEP, an inlined typed dictionary is a also a class, with an empty string for__name__
(which isn't ideal to be fair). Initially, I wanted inlined typed dictionaries to be an instance of some_InlinedTypedDict
class, but this complicates the runtime behavior. For example, should we allow accessing the introspection attributes?