Skip to content

Change the rules for structural subtyping of TypedDicts:… #12142

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

Conversation

bsmedberg-xometry
Copy link

@bsmedberg-xometry bsmedberg-xometry commented Feb 7, 2022

… a TypedDict with a more-specific type is now considered a subtype of a TypedDict with a less-specific type.

Examples:

  • {'a': str} is now a subtype of {'a': NotRequired[str]}
  • {'a': str} is now a subtype of {'a': Optional[str]}

This PR came up in the context of #12095 - in that PR I wrote a narrowing pass that would narrow the required properties of typed dicts with code that checked properties like "key" in td. Except that was causing errors because the narrowed type was considered incompatible with the original type.

This does somewhat alter the ergonomics and safety guarantees of the type system: because you could then do things like mutate the more generic type and make changes that were incompatible with the specific type. However in most structural-subtyping systems (including typescript), you're allowed to convert from a more-specific type to the less-specific type, with the understanding that this could allow type-incorrect mutations to occur.

Test Plan

Automated/unit tests cover the primary functionality of this PR. I await the mypy_primer results: I do not expect any errors as this PR is strictly looser than previously, but it's possibly that type-ignore comments are no longer required in some places.

@davidfstr @JukkaL tagging you because of your prior relationship with TypedDict typing and this code.

…ecific

type is now considered a subtype of a TypedDict with a less-specific type.

Examples:
* `{'a': str}` is now a subtype of `{'a': NotRequired[str]}`
* `{'a': str}` is now a subtype of `{'a': Optional[str]}`
@github-actions
Copy link
Contributor

github-actions bot commented Feb 7, 2022

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JelleZijlstra
Copy link
Member

This is unsafe:

class A(TypedDict):
    a: str
class OptA(TypedDict):
    a: str | None
class NRA(TypedDict):
    a: NotRequired[str]

a: A = {"a": "b"}

def take_opt_a(d: OptA) -> None:
    d["a"] = None  # ok, "a" can be None
def take_nra(d: NRA) -> None:
    d.pop("a", None)  # ok, "a" can be deleted

take_opt_a(a)
# oops, now a["a"] is None

take_nra(a)
# oops, now a["a"] doesn't exist

To be fair these don't seem likely to occur in real code, but they do create a hole in the type system.

@davidfstr
Copy link
Contributor

This is unsafe:

Agreed. The original design of TypedDict explicitly disallows the subtyping relationships mentioned above because they are unsafe. So I am hard -1 on this PR as-is.

That being said, I expect it is common for users of TypedDict to use it in read-only contexts. In such read-only contexts it is safe to have the subtyping rules mentioned above. We just need a way to spell "a read-only TypedDict". Consider:

class A(TypedDict, readonly=True):
    a: str
class OptA(TypedDict, readonly=True):
    a: str | None
class NRA(TypedDict, readonly=True):
    a: NotRequired[str]

a: A = {"a": "b"}

def write_to_opt_a(d: OptA) -> None:
    d["a"] = None  # ERROR: OptA is read-only and does not permit mutation
def write_to_nra(d: NRA) -> None:
    d.pop("a", None)  # ERROR: NRA is read-only and does not permit mutation

def read_from_opt_a(d: OptA) -> str | None:
    return d["a"]  # ok
def read_from_nra(d: NRA) -> str | None:
    return d.get("a")

print(read_from_opt_a(a))  # prints "b"
print(read_from_nra(a))  # prints "b"

@bsmedberg-xometry , if this type of subtyping relationship is especially important to you, you might consider pursuing the design/implementation of a "read-only TypedDict" in a new PEP. (Other readers, of course, may also be implementing such a PEP too.)

@bsmedberg-xometry
Copy link
Author

Going to close this pull request, because it clearly doesn't match the mypy (python?) typing design philosophy. I may eventually write a PEP for explicitly marking function arguments and other variables as covariant/contravariant (currently this is only possible with generics), but it's not necessary to solve my most urgent issues.

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.

3 participants