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

Required/NotRequired don't work with __future__.annotations #55

Open
jcrist opened this issue Jul 11, 2022 · 7 comments
Open

Required/NotRequired don't work with __future__.annotations #55

jcrist opened this issue Jul 11, 2022 · 7 comments

Comments

@jcrist
Copy link

jcrist commented Jul 11, 2022

Required/NotRequired aren't properly detected when from __future__ import annotations is used, leading to incorrect __required_keys__/__optional_keys__ sets.

Reproduced with 4.3.0.

from __future__ import annotations  # comment this line out and things work correctly
from typing_extensions import NotRequired, TypedDict

class Test(TypedDict):
    a: int
    b: NotRequired[int]

assert Test.__required_keys__ == {"a"}
assert Test.__optional_keys__ == {"b"}
$ python test.py
Traceback (most recent call last):
  File "/home/jcristharif/Code/test.py", line 8, in <module>
    assert Test.__required_keys__ == {"a"}
AssertionError
@JelleZijlstra
Copy link
Member

What Python version are you running? Does this reproduce with typing too on Python 3.11?

@jcrist
Copy link
Author

jcrist commented Jul 11, 2022

I don't have (easy) access to a 3.11 build, but it reproduces with 4.3.0 on both 3.10 and 3.9.

@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 12, 2022

I can reproduce using typing.TypedDict and typing.NotRequired on the CPython main branch (3.12), so I assume it reproduces on 3.11 using the stdlib typing module as well.

@yixun-alpha
Copy link

just ran into this issue with 4.3.0 and python 3.7.6 - any fix in the works? thanks!

@gvanrossum
Copy link
Member

So is this fixed by #60 or not? It seems to be an issue with the upstream cpython typing module as well -- did someone already file an issue there? Please link it here.

@JelleZijlstra
Copy link
Member

#60 would fix this in simple cases, but I think it will cause more problems than it solves. I'm inclined to wontfix this. Maybe we can revisit once there's a resolution on the PEP 563/649 saga.

@gvanrossum
Copy link
Member

It's fine to not want to solve this for now. In fact one could argue that, since the point of from __future__ import annotations is not to incur runtime costs, you get what you ask for. (Not sure if that's totally fair, but it's a possible POV.)

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 a pull request may close this issue.

5 participants