Skip to content
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

TypedDict: __required_keys__ and __optional_keys__ can be wrong in the presence of inheritance #112509

Closed
JelleZijlstra opened this issue Nov 29, 2023 · 5 comments
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@JelleZijlstra
Copy link
Member

JelleZijlstra commented Nov 29, 2023

Bug report

Bug description:

from typing import NotRequired, Required, TypedDict

class A(TypedDict):
    a: NotRequired[int]
    b: Required[int]

class B(A):
    a: Required[int]
    b: NotRequired[int]

print(B.__required_keys__)
print(B.__optional_keys__)

This will print (tried on 3.11 and current-ish main):

frozenset({'b', 'a'})
frozenset({'b', 'a'})

But obviously, a single key should only be either Required or NotRequired, not both. The child class's version should prevail.

@alicederyn and I discovered this while discussing the implementation of PEP-705 (python/typing_extensions#284). cc @davidfstr for PEP 655.

I also see some problems with how type checkers handle this case, but i'll post about that separately.

CPython versions tested on:

3.11, CPython main branch

Operating systems tested on:

macOS

Linked PRs

@JelleZijlstra JelleZijlstra added type-bug An unexpected behavior, bug, or error 3.11 only security fixes topic-typing 3.12 bugs and security fixes 3.13 bugs and security fixes labels Nov 29, 2023
JelleZijlstra added a commit to JelleZijlstra/cpython that referenced this issue Nov 29, 2023
@alicederyn
Copy link

This example is illegal according to PEP-589.

A TypedDict type with a required key is not consistent with a TypedDict type where the same key is a non-required key, since the latter allows keys to be deleted.

Concretely, B cannot be a subclass of A as it "removes" the ability to delete key 'b' (breaking substitutability) and also adds None to the possible values returned by get('a') (also breaking substitutability).

@AlexWaygood AlexWaygood added the stdlib Python modules in the Lib dir label Nov 29, 2023
@JelleZijlstra
Copy link
Member Author

@alicederyn you may be right that this is illegal at type checking time; I posted python/typing#1516 about this. But as Alex pointed out on the PR, backward compatibility means we can't just prohibit this at runtime. PEP 705 is actually a good illustration of why it's good to be lenient at runtime: it creates a mechanism that does let you combine NotRequired and Required for the same key in a type-safe manner. If we had prohibited that at runtime, it would make it harder for people to use PEP 705 on older Python versions.

JelleZijlstra added a commit that referenced this issue Nov 29, 2023
…keys in TypedDict (#112512)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 29, 2023
…ional_keys in TypedDict (pythonGH-112512)

(cherry picked from commit 4038869)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Nov 29, 2023
…ional_keys in TypedDict (pythonGH-112512)

(cherry picked from commit 4038869)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
JelleZijlstra added a commit that referenced this issue Nov 29, 2023
…tional_keys in TypedDict (GH-112512) (#112531)

(cherry picked from commit 4038869)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
JelleZijlstra added a commit that referenced this issue Nov 29, 2023
…tional_keys in TypedDict (GH-112512) (#112530)

(cherry picked from commit 4038869)

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@alicederyn
Copy link

alicederyn commented Nov 29, 2023

If we had prohibited that at runtime, it would make it harder for people to use PEP 705 on older Python versions.

I wasn't intending to suggest prohibiting it at runtime -- just that changing the runtime behaviour of an unsupported construction seems unnecessary. I don't think there's a downside to changing it per se, but I don't think the previous behaviour was a bug.

Using PEP-705 with a regular TypedDict instead of importing it from typing_extensions will give the wrong behaviour at runtime anyway (the type qualifier will incorrectly end up mixed with the types at best, at worst it will accidentally break which keys are declared as required/not required, depending on what order you use the qualifiers in), so I don't think this change will benefit anyone there.

@AlexWaygood
Copy link
Member

This is fixed at runtime now, and backported to 3.11+, so I'll close this out now

@JelleZijlstra
Copy link
Member Author

@alicederyn that's a fair point, but I think the behavior here (the same key being in both required and optional keys) is quite clearly buggy; there is room for disagreement (or future spec changes) in the type checker behavior; and the fix was simple.

aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
…ional_keys in TypedDict (python#112512)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Glyphack pushed a commit to Glyphack/cpython that referenced this issue Sep 2, 2024
…ional_keys in TypedDict (python#112512)

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 bugs and security fixes 3.13 bugs and security fixes stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants