-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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: Updates from discussion #4270
base: main
Are you sure you want to change the base?
Conversation
peps/pep-0764.rst
Outdated
|
||
Movie = TypedDict[{'name': NotRequired[str], 'year': ReadOnly[int]}] | ||
|
||
Inlined typed dictionaries are implicitly *total*, meaning all keys must be | ||
present. Using the :data:`~typing.Required` type qualifier is thus redundant. | ||
|
||
If :pep:`728` gets accepted, inlined typed dictionaries will be implicitly |
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 implies an ordering dependency between the two PEPs. If this PEP is accepted prior to 728, then this would represent a breaking change if and when PEP 728 is accepted — something that will likely get significant pushback. Therefore, it's probably best for this PEP to be considered for acceptance only after (or concurrently with) PEP 728's submission.
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.
Agree, that's why I initially put this in the open issues section. I'll move it back there, and add a note that PEP 728 needs to be addressed (either accepted or rejected) before pursuing with PEP 764.
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 hoping to push PEP 728 to resolution soon.
peps/pep-0764.rst
Outdated
@@ -137,11 +139,19 @@ are bound to some outer scope:: | |||
|
|||
InlinedTD = TypedDict[{'name': T}] # Not OK, `T` refers to a type variable that is not bound to any scope. | |||
|
|||
|
|||
It is not possible for an inlined typed dictionary to be extended:: |
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.
Why is extending an inlined TypedDict disallowed? I understand why we'd want to disallow extending other TypedDicts via an inlined TypedDict definition, but it's unclear why this limitation would be imposed. It seems kind of arbitrary. Perhaps there's some complication related to runtime implementation?
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.
So from a static type checking perspective, I don't think it matters really, although the usefulness is debatable. However, depending on the runtime implementation we choose to go to (either a normal TypedDict
class with an arbitrary '<inlined TypedDict>'
name, or an instance of a new typing._TBDClass
), the runtime behavior might be difficult to implement. I'll move this in the open issues section, alongside the runtime implementation discussion.
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 it matters really, although the usefulness is debatable
I think we should strive to make new features consistent. Regardless of how a TypedDict is defined (the class
syntax, functional syntax, or inlined syntax), the resulting type should work the same. If this limitation is left in the PEP, it means that there will be two variants of TypedDict that act differently — and therefore need to be treated by static analysis tools as slightly different beasts.
peps/pep-0764.rst
Outdated
@@ -137,11 +139,19 @@ are bound to some outer scope:: | |||
|
|||
InlinedTD = TypedDict[{'name': T}] # Not OK, `T` refers to a type variable that is not bound to any scope. |
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 with the text in this section, but the example code looks incorrect to me. The example shows a generic type alias declaration using the old-style (PEP 747) way of defining a type alias. In this case, the type variable T
is bound to a valid scope — the type alias.
You can address this by changing the example code to use a form that will not be interpreted by a type checker as a type alias. For example, use a local variable (within a function body) or add a type annotation.
def func():
InlinedTD = TypedDict[{"name": T}]
InlinedTD: object = TypedDict[{"name": T}]
typing.Dict
: same reasoning asdict
PEP 123: Summary of changes
)📚 Documentation preview 📚: https://pep-previews--4270.org.readthedocs.build/