gh-137226: Fix get_type_hints() on generic TypedDict with stringified annotations #138953
Conversation
…gified annotations This issue appears specifically for TypedDicts because the TypedDict constructor code converts string annotations to ForwardRef objects, and those are not evaluated properly by the get_type_hints() stack because of other shenanigans with type parameters. This issue does not affect normal generic classes because their annotations are not pre-converted to ForwardRefs. The fix attempts to restore the pre- python#137227 behavior in the narrow scenario where the issue manifests. It mostly makes changes only in the paths accessible from get_type_hints(), ensuring that newer APIs (such as evaluate_forward_ref() and annotationlib) are not affected by get_type_hints()'s past odd choices. This PR does not fix python#138949, an older issue I discovered while playing around with this one; we'll need a separate and perhaps more invasive fix for that, but it should wait until after 3.14.0.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
LGTM. Some small nits/questions but nothing blocking; can always merge this and address this later if @hugovk wants it in ASAP!
|
|
||
| def _eval_type(t, globalns, localns, type_params, *, recursive_guard=frozenset(), | ||
| format=None, owner=None, parent_fwdref=None): | ||
| format=None, owner=None, parent_fwdref=None, prefer_fwd_module=False): |
There was a problem hiding this comment.
Could we add a description of this parameter to the docstring?
Should this default to True? Third-party users of this function may be expecting the legacy get_type_hints behaviour? They're obviously asking for breakage if they're using an undocumented, private implementation detail though. So I don't have a strong opinion here.
There was a problem hiding this comment.
I think the False behavior is more sensible so I want to default to it. I know this function gets used in the wild and we shouldn't go out of our way to break such uses, but at the end of the day it's a private function.
|
Thanks @JelleZijlstra for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
|
Sorry, @JelleZijlstra, I could not cleanly backport this to |
…h stringified annotations (pythonGH-138953) This issue appears specifically for TypedDicts because the TypedDict constructor code converts string annotations to ForwardRef objects, and those are not evaluated properly by the get_type_hints() stack because of other shenanigans with type parameters. This issue does not affect normal generic classes because their annotations are not pre-converted to ForwardRefs. The fix attempts to restore the pre- pythonGH-137227 behavior in the narrow scenario where the issue manifests. It mostly makes changes only in the paths accessible from get_type_hints(), ensuring that newer APIs (such as evaluate_forward_ref() and annotationlib) are not affected by get_type_hints()'s past odd choices. This PR does not fix issue pythonGH-138949, an older issue I discovered while playing around with this one; we'll need a separate and perhaps more invasive fix for that, but it should wait until after 3.14.0. (cherry picked from commit 6d6aba2) Co-authored-by: Jelle Zijlstra <[email protected]>
|
GH-138989 is a backport of this pull request to the 3.14 branch. |
4 participants
This issue appears specifically for TypedDicts because the TypedDict constructor
code converts string annotations to ForwardRef objects, and those are not evaluated
properly by the get_type_hints() stack because of other shenanigans with type
parameters.
This issue does not affect normal generic classes because their annotations are not
pre-converted to ForwardRefs.
The fix attempts to restore the pre- #137227 behavior in the narrow scenario where
the issue manifests. It mostly makes changes only in the paths accessible from get_type_hints(),
ensuring that newer APIs (such as evaluate_forward_ref() and annotationlib) are not affected
by get_type_hints()'s past odd choices. This PR does not fix issue #138949, an older issue I
discovered while playing around with this one; we'll need a separate and perhaps more
invasive fix for that, but it should wait until after 3.14.0.