bpo-42904: Fix get_type_hints for class local namespaces#24201
Conversation
|
This PR is stale because it has been open for 30 days with no activity. |
| base_locals = dict(vars(base)) if localns is None else localns | ||
| if localns is None and globalns is None: | ||
| # This is surprising, but required. Before Python 3.10, | ||
| # get_type_hints only evaluated the globalns of | ||
| # a class. To maintain backwards compatibility, we reverse | ||
| # the globalns and localns order so that eval() looks into | ||
| # *base_globals* first rather than *base_locals*. | ||
| base_globals, base_locals = base_locals, base_globals |
There was a problem hiding this comment.
Ow, this is indeed weird. I guess it means that if we have
class A: ...
class B:
x: "A"
class A: ...
then get_type_hints(B)["x"] will refer to the global A, not to B.A?
I suppose you ran into tests that broke if you didn't swap the variables, and those tests did have both a global and a local definition of a type used in an annotation with the same name? But this seems arbitrary. I tried annotations with and without quotation marks, and before and after the inner 'class A:', and now they all refer to the global A. But arguably they could all refer to B.A.
What do you think? (Specifically what tests would fail without this counter-intuitive swap?)
There was a problem hiding this comment.
I remember some tests failing if I didn't have this swap. However, now when I remove the swap, all the tests miraculously pass. Perhaps the past me of 3 months ago saw something that the current me doesn't. But now I have no clue what I was doing, so I removed it.
The magic of time allows for tests to magically pass I guess 😕?
gvanrossum
left a comment
gvanrossum
left a comment
There was a problem hiding this comment.
Glad that was a non-issue! Approved. I will merge it now.
|
Does the test for this need to change, given that stringized annotations are no longer the default in Python 3.10? If I understand it correctly, the text in this PR no longer calls |
Larry, you're absolutely right! Thanks for catching this. I created #25632 to address that (it's just a teeny edit). Edit: Woops I was wrong, it's not going to be a teeny edit at all :(. |
5 participants
Currently this code errors:
Now it works. An oddity with
get_type_hints(noted by others on python-dev) is that it previously seemed to do nothing at all to find the local namespace of a class...https://bugs.python.org/issue42904