bpo-41370: Evaluate strings as forward refs in PEP 585 generics#30900
bpo-41370: Evaluate strings as forward refs in PEP 585 generics#30900gvanrossum merged 10 commits into
Conversation
…`ForwardRef` before evaluation (https://bugs.python.org/issue41370)
This comment has been minimized.
This comment has been minimized.
JelleZijlstra
left a comment
There was a problem hiding this comment.
Looks pretty good! I have one small suggestion. This also needs a NEWS entry, which you can add with blurb.
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
This is the code that I posted to the issue. But I also posted a few questions there, and those should at least be answered by tests:
-
But would that be enough? The algorithm would have to recursively dive into args to see if there's a string hidden deep inside, e.g. list[tuple[int, list["N"]]].
-
And what if the user writes something hybrid, like List[list["N"]]? What other cases would we need to cover?
-
And can we sell this as a bugfix for 3.10, or will this be a new feature in 3.11?
-
How will it interact with from future import annotations?
Sorry, something went wrong.
The tests already cover the sort of case you list here (there's a dict[int, list[List[list["T"]]] I think). We could add more variants though.
I think this can be considered a bugfix. get_type_hints() is supposed to evaluate forward references but it didn't in all cases.
That's another interesting test case. It would basically give us two levels of stringification, like |
Sorry, something went wrong.
|
Would also be good to test a recursive forward ref: We guard against this sort of thing already, but we should validate it works in this case too. |
Sorry, something went wrong.
|
You can add a NEWS entry using https://blurb-it.herokuapp.com/ as an alternative to the CLI :) |
Sorry, something went wrong.
Yes, sorry for not mentioning it. Same code, different location. I've added two more unit tests for the cases Guido and you mentioned. Thanks, added it. |
Sorry, something went wrong.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
|
Actually I'm not sure if we can find a good way to make the |
Sorry, something went wrong.
We can just change those tests from |
Sorry, something went wrong.
gvanrossum
left a comment
There was a problem hiding this comment.
LGTM.
I'll wait landing this until 3.11a6 is out, so as not to disturb the release process (which is already stressed by a last-minute blocker).
Sorry, something went wrong.
|
There's no C code here, and 3.11a6 seems to be still delayed, so I'm just landing this. Sorry for the confusion. |
Sorry, something went wrong.
|
Update: This didn't make 3.11a6. The release process was farther along than I realized. |
Sorry, something went wrong.
This Pull Requests suggests a change to
typing._eval_type()that considers strings in PEP 585 generics (ie. instances oftypes.GenericAlias) as forward references. This is necessary becauseForwardRefis not and likely will not be implemented in C, and thus strings used as forward references in PEP 585 are not currently evaluated bytyping.get_type_hints().https://bugs.python.org/issue41370
A test case that uses
assertIs()currently fails because in the current state_eval_type()creates a copy of the same generic alias with transformed arguments, thus for any PEP 585 generic aliasx, the comparisonx is typing.get_type_hints(func)['X']will evaluate toFalse, assumingfunchas a field/argument annotationX: x. I think that this can be worked around, and happy to propose an updated implementation.I created this PR as a proof of concept until a there is consent that in spirit this change to
get_type_hints()is something that should happen. So for now I'll keep it as it is.Open todos/questions
test_get_type_hints_annotatedintest_typing.pytypes.GenericAliasseparately or is it safe to assume that a string argument is always a forward reference?Co-Authored-By: Guido van Rossum guido@python.org
https://bugs.python.org/issue41370