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

__future__ annotations breaks TypedDict __required/optional_keys__ #97727

Open
scop opened this issue Oct 2, 2022 · 7 comments
Open

__future__ annotations breaks TypedDict __required/optional_keys__ #97727

scop opened this issue Oct 2, 2022 · 7 comments
Labels
stdlib Python modules in the Lib dir topic-typing type-bug An unexpected behavior, bug, or error

Comments

@scop
Copy link
Contributor

scop commented Oct 2, 2022

Bug report

from __future__ import annotations appears to break TypedDict required/optional keys ending up in __required_keys__ and __optional_keys__. mypy works as expected though.

Using the example from https://peps.python.org/pep-0655/#usage-in-python-3-11 as the base

$ cat t.py
from __future__ import annotations

from typing_extensions import NotRequired, TypedDict

class Dog(TypedDict):
    name: str
    owner: NotRequired[str]

print("required", Dog.__required_keys__)
print("optional", Dog.__optional_keys__)
$ python3 t.py
required frozenset({'name', 'owner'})
optional frozenset()

With the __future__ import removed, works as expected:

$ cat t.py
from typing_extensions import NotRequired, TypedDict

class Dog(TypedDict):
    name: str
    owner: NotRequired[str]

print("required", Dog.__required_keys__)
print("optional", Dog.__optional_keys__)
$ python3 t.py
required frozenset({'name'})
optional frozenset({'owner'})

Note: breaks across different variations of total and Required/NotRequired and typing_extensions vs typing imports, above is just one example.

https://peps.python.org/pep-0655/#how-to-teach-this contains an example with the __future__ annotations import in place with no mention that it would not cause __required_keys__ and __optional_keys__ becoming populated as expected, so I'm assuming this is a bug.

Your environment

  • CPython versions tested on:

    • 3.9.7 + NotRequired and TypedDict imports from typing_extensions
    • 3.10.7 + above mentioned imports from typing_extensions
    • current 3.11.0rc2+ + above mentioned imports from typing_extensions
    • current 3.11.0rc2+ above mentioned imports from typing
  • Operating system and architecture: Linux x86_64

@scop scop added the type-bug An unexpected behavior, bug, or error label Oct 2, 2022
@JelleZijlstra
Copy link
Member

This was also reported in python/typing_extensions#55. As discussed there, it's hard to find a solution for this problem that doesn't cause new problems, because the most obvious solution (evaluating the annotations when the TypedDict is defined) would break various use cases for forward references.

@martindemello
Copy link
Contributor

from a pragmatic point of view, i wonder if we can add a pure string parsing function that unwraps the outer container (if any) of a forward ref. then typeddict here could consider the tuple (ForwardRef('NotRequired'), ForwardRef('str')) and possibly choose to resolve the first element, or simply apply some heuristics to see if it would resolve to typing_extensions.NotRequired

@hmc-cs-mdrissi
Copy link
Contributor

The main issue with string heuristics is deciding which of these cases you want to handle,

from typing import Required as R, NotRequired as NR, Annotated
import typing as t

class Foo(TypedDict):
  x: R[int]
  y: NR[str]
  z: Annotated[NR[str], ...]
  w: Annotated[t.Required[str], ...]

Dataclasses does do a similar thing for ClassVar, so you could pick which cases to support and document it. I'd view this as an improvement over status quo, although it'd stay a subtle thing.

One way to avoid most breakage involving forward refs would be delaying get_type_hints call to time required_keys is accessed by having it be like a class property. Using get_annotations maybe to just have same behavior of letting globals be modules namespace and locals be class namespace. Rough code idea,

from __future__ import annotations

from inspect import get_annotations
from typing import get_origin

from typing_extensions import NotRequired, Required


class LazyTypedDict:
    @classmethod
    def required_keys(cls):
        explicit_required_keys = {k for k, v in get_annotations(cls, eval_str=True).items() if get_origin(v) is Required}
        return explicit_required_keys

class Foo(LazyTypedDict):
    a: Required[int]
    b: int
    c: NotRequired[int]

print(Foo.required_keys()) # Prints {"a"}

This is enough to handle stuff like recursive/mutual recursion just fine too.

class RecA(LazyTypedDict):
    b: Required[ReqB]

class ReqB(LazyTypedDict):
    a: Required[RecA]
    s: int

print(ReqB.required_keys()) # Prints {"a"} as wanted.

I'd be interested in doing a pr for either approach. Would string heuristics + caveats or lazy evaluation be preferred?

@JelleZijlstra
Copy link
Member

I'm hoping that the SC will soon come to a decision on whether to go with PEP 563 vs. 649. Once that's done we, may get more clarity on the right approach to problems like this one.

@scop
Copy link
Contributor Author

scop commented Oct 5, 2022

Thanks for considering. Perhaps in the meantime it would be good to add a caveat note to https://peps.python.org/pep-0655/#how-to-teach-this about the possible breakage.

@JelleZijlstra
Copy link
Member

PEP-649 has now been accepted, rendering from __future__ import annotations essentially deprecated. Therefore, I think we should close this as "won't fix" instead of introducing any string-based hacks.

@AlexWaygood
Copy link
Member

AlexWaygood commented May 22, 2023

PEP-649 has now been accepted, rendering from __future__ import annotations essentially deprecated. Therefore, I think we should close this as "won't fix" instead of introducing any string-based hacks.

I think it's fine to say we won't fix this on the basis that it's too complex. I don't think PEP-649's acceptance is a good rationale to say we won't fix this, however. For code bases that want to support all non-EOL Python versions, from __future__ import annotations will likely continue to be used for a long time after PEP 649 has been implemented in Python 3.13. Differences of behaviour under PEP-563 are therefore likely to impact a large number of users for a while yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

5 participants