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 'in' narrowing w/o @final #15697

Open
ikonst opened this issue Jul 18, 2023 · 4 comments · May be fixed by #15698
Open

TypedDict 'in' narrowing w/o @final #15697

ikonst opened this issue Jul 18, 2023 · 4 comments · May be fixed by #15698
Labels

Comments

@ikonst
Copy link
Contributor

ikonst commented Jul 18, 2023

We shouldn't require @final decoration for TypedDicts to narrow them based on the 'in' operator.

Why?

Basically @erictraut's comment.

In #13838, we've added "key in Union[TypedDict, ...]" narrowing for TypedDicts that are marked @final. The reason was to prevent this:

class Mammal(TypedDict):
  mammary_glands: int

class Bird(TypedDict):
  eggs: int

class Echidna(Mammal):
  eggs: int

animal: Mammal | Bird
if 'eggs' in animal:
  assert_type(animal, Bird)  # WRONG! Could still be a Mammal (a Echidna)
if 'eggs' in animal and 'mammary_glands' in animal:
  assert_never(animal)  # WRONG! Could still be a Mammal (a Echidna)

However, per @erictraut's comment, due to TypedDict being a structural type, we shouldn't consider the class hierarchy when type-matching.

This will be consistent with pyright and TypeScript (Playground Link).

@A5rocks
Copy link
Contributor

A5rocks commented Jul 18, 2023

Just a note that perhaps, the current behavior can be patched in another (if worse) way?

#7981 (comment) as mentioned here, final typeddict semantics was accepted knowing that you wouldn't be able to consider structural subtyping... as far as I read it?

i.e. this should fail (it doesn't right now):

from typing import TypedDict, final

@final
class A(TypedDict):
    x: int

class B(TypedDict):
    x: int
    y: int

y: B = {"x": 42, "y": 42}
x: A = y

... but at the same time #15425 was accepted so this new unsafety is probably ok.

@ikonst
Copy link
Contributor Author

ikonst commented Sep 2, 2023

@hauntsaninja Judging by your comment here, let's ship it? :)

It would appear that's also how people expect it to be.

@gvanrossum
Copy link
Member

FWIW there are some typos in the example -- mypy doesn't accept it as written. The last lines probably were intended to be:

animal: Mammal | Bird
if 'eggs' in animal:
  assert_type(animal, Bird)  # WRONG! Could still be a Mammal (a Echidna)
if 'eggs' in animal and 'mammary_glands' in animal:
  assert_never(animal)  # WRONG! Could still be a Mammal (a Echidna)

@ikonst
Copy link
Contributor Author

ikonst commented Jan 21, 2024

Thanks @gvanrossum, edited the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants