Skip to content

PEP 728: Incorporate feedback since last revision #4380

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

Merged
merged 3 commits into from
Apr 22, 2025

Conversation

PIG208
Copy link
Contributor

@PIG208 PIG208 commented Apr 18, 2025


📚 Documentation preview 📚: https://pep-previews--4380.org.readthedocs.build/

- Mention indexed accesses and assignments with arbitrary keys,
  per Carl's feedback.

- Prepare for submission by clearing the "open issues" sections, with
  edits when needed.

- Include a reference to a prior discussion on `@final`
  addressing Victorien feedback
  (https://discuss.python.org/t/pep-728-typeddict-with-typed-extra-items/45443/138).

- Update reference implementations.

Signed-off-by: Zixuan James Li <[email protected]>
@PIG208 PIG208 requested a review from JelleZijlstra as a code owner April 18, 2025 04:44
@PIG208 PIG208 changed the title PEP 728: Incorporate discussions since last revision PEP 728: Incorporate feedback since last revision Apr 18, 2025
@Viicos
Copy link
Contributor

Viicos commented Apr 18, 2025

Thanks for addressing part of my feedback. Regarding my other points:

Alternatively, the PEP could emphasize on the fact that closed typed dictionaries are only relevant in a context where assignability of arguments to parameters is involved (as “simple” assignments (a: = value) seems to be special cased)

I'll try to get confirmation about this but it's maybe not worth being mentioned in the PEP.

It is stated that closed=True is equivalent to extra_items=Never. Does this mean that closed=False (the default) is equivalent to extra_items=object? This currently not the case (playground)

I'm not sure how this could be clarified, but it does look confusing. It might be related to my first point though, as the playground example uses "simple" assignments.

Signed-off-by: Zixuan James Li <[email protected]>
@PIG208
Copy link
Contributor Author

PIG208 commented Apr 19, 2025

Thanks for the reviews and followups! Pushed a new revision.

Re: @Viicos follow-ups

I think the special case you mentioned (also discussed in this section) is a result of TypedDicts following the existing structural assignabillity rules.

This section on extra_items shows some examples where this special case no longer applies, but only when extra_items is specified.


The intention of having closed=False is to preserve the current TypedDict behavior (which includes the special check during construction). This part is addressed in a paragraph from this section:

When neither extra_items nor closed=True is specified, the TypedDict
is assumed to allow non-required extra items of value type ReadOnly[object]
during inheritance or assignability checks. This preserves the existing behavior
of TypedDict.

I think we can also mention that closed=False is assumed here, and the check for extra keys during construction (this check is from the current typing spec).

@Andrej730
Copy link
Contributor

Noticed a small typo :

Practiaclly, the key collision with a regular key can be mitigated with

Also, isn't it the same thing mentioned twice?

peps/peps/pep-0728.rst

Lines 206 to 208 in 95793de

The alternative inline syntax is also supported::
Movie = TypedDict("Movie", {"name": str}, extra_items=bool)

peps/peps/pep-0728.rst

Lines 236 to 238 in 95793de

``extra_items`` is also supported with the functional syntax::
Movie = TypedDict("Movie", {"name": str}, extra_items=int | None)

@PIG208
Copy link
Contributor Author

PIG208 commented Apr 20, 2025

Thanks! Pushed another update to address @Andrej730's comments.

@JelleZijlstra JelleZijlstra merged commit a8a31fe into python:main Apr 22, 2025
5 checks passed
@JelleZijlstra
Copy link
Member

Thanks! I just found that this has already been submitted to the Typing Council (python/typing-council#22); I'll ping the other members so we can finally come to a decision.

@PIG208 PIG208 deleted the revised branch April 22, 2025 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants