-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
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
gh-114053: Fix bad interaction of PEP-695, PEP-563 and get_type_hints
#118009
Conversation
AlexWaygood
commented
Apr 17, 2024
•
edited by bedevere-app
bot
Loading
edited by bedevere-app
bot
- Issue: typing.get_type_hints fails when passed a class with PEP 695 type parameters and PEP 563 is enabled #114053
Lib/typing.py
Outdated
eval( | ||
self.__forward_code__, | ||
globalns, | ||
{param.__name__: param for param in type_params} | localns |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this means that any annotations in the "local namespace" of the class override any symbols that are present in the scope due to being listed as a type parameter. That matches the behaviour of __annotations__
at runtime in a module that doesn't use __future__
annotations:
>>> class B[T, *Ts, **P]:
... T = int
... Ts = str
... P = bytes
... x: T
... y: Ts
... z: P
...
>>> B.__annotations__
{'x': <class 'int'>, 'y': <class 'str'>, 'z': <class 'bytes'>}
I'm doing it at the last possible point here as it feels less risky than doing it in _eval_type
.
Doc/library/typing.rst
Outdated
them in ``globals``, ``locals`` and (where applicable) | ||
:ref:`type parameter <annotation-scopes>` namespaces. | ||
For a class ``C``, return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't usually add .. versionchanged
notes for bugfixes (and I think this is a bugfix), but I could add one if we think this is a significant enough change to the function's semantics
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a much simpler fix than I was afraid we'd need.
Co-authored-by: Jelle Zijlstra <[email protected]>
Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Sorry, @AlexWaygood, I could not cleanly backport this to
|
GH-118104 is a backport of this pull request to the 3.12 branch. |
…`get_type_hints`` (python#118009) (cherry-picked from commit 1e3e7ce) Co-authored-by: Jelle Zijlstra <[email protected]>
…ype_hints`` (#118009) (#118104) Co-authored-by: Jelle Zijlstra <[email protected]>
In CPython 3.12.4/3.13, the function signature of `FutureRef._evaluate` changed such that `recursive_guard` is no longer a positional argument; it is now keyword only [0][1]. To accomodate this, specify `recursive_guard` as a kwarg. This syntax is backwards compatible with earlier versions of the function signature. [0]: python/cpython#118104 [1]: python/cpython#118009 Signed-off-by: Vincent Fazio <[email protected]>
In CPython 3.12.4/3.13, the function signature of `FutureRef._evaluate` changed such that `recursive_guard` is no longer a positional argument; it is now keyword only [0][1]. To accommodate this, specify `recursive_guard` as a kwarg. This syntax is backwards compatible with earlier versions of the function signature. [0]: python/cpython#118104 [1]: python/cpython#118009 Signed-off-by: Vincent Fazio <[email protected]>
`recursive_guard` got made keyword arg only in python/cpython#118009 which released in `3.12.4` (we run an older version in buildkite so we missed it) fixes #22985 ## How I Tested These Changes on a fresh 3.12.4 venv `pytest python_modules/dagster/dagster_tests/general_tests/test_record.py python_modules/dagster/dagster_tests/general_tests/check_tests/test_check.py` which was failing before fix
cc for 3.12 backport: https://discuss.python.org/t/3-12-3-3-12-4-regression-deep-rabbit-hole-warning/59584/12 |