Repository navigation
PEP 585 and ForwardRef #85542
Description
Activity
wyfo commented
on Jul 22, 2020 wyfomannequinMannequinAuthorPEP-585 current implementation (3.10.0a0) differs from current Generic implementation about ForwardRef, as illustrated bellow:
from dataclasses import dataclass, field from typing import get_type_hints, List, ForwardRef @dataclass class Node: children: list["Node"] = field(default_factory=list) children2: List["Node"] = field(default_factory=list) assert get_type_hints(Node) == {"children": list["Node"], "children2": List[Node]} assert List["Node"].__args__ == (ForwardRef("Node"),) assert list["Node"].__args__ == ("Node",) # No ForwardRef here, so no evaluation by get_type_hints
There is indeed no kind of ForwardRef for
listarguments. As shown in the example, this affects the result of get_type_hints for recursive types handling.He could be "fixed" in 2 lines in
typing._eval_typewith something like this :def _eval_type(t, globalns, localns, recursive_guard=frozenset()): if isinstance(t, str): t = ForwardRef(t) if isinstance(t, ForwardRef): ...
but it's kind of hacky/dirty.
It's true that this issue will not concern legacy code, 3.9 still being not released. So developers of libraries using get_type_hints could add in their documentation that
from __future__ import annotationsis mandatory for recursive types with PEP-585 (I think I will do it).By the way, Guido has quickly given his opinion about it in PR 21553: "We probably will not ever support this: importing ForwardRef from the built-in generic alias code would be problematic, and once from __future__ import annotations is always on there's no need to quote the argument anyway." (So feel free to close this issue)
I think mentioning this in the docs is the best we can do in 3.9, and for 3.10 the point will be moot. The next release is release candidate 1, so we're well past the point where we can implement new functionality.
wyfo commented
on Jul 22, 2020 wyfomannequinMannequinAuthorHowever, PEP-563 will not solve the recursive type alias issue like
A = list["A"]but this is a minor concern.Hm, recursive type aliases are an interesting issue. We may be able to do better there for 3.10, even if we can't fix it for 3.9 (or at least not for 3.9.0).
But in the meantime maybe you can come up with a PR that adds a note to the typing docs in 3.10 explaining that
list["int"]will not be resolved tolist[int], even though it works forList["int"]?NiklasRosenstein commented
on Jan 7, 2022 NiklasRosensteinmannequinMannequinI'm running into this issue right now. Can anyone provide a rationale as to why you think this is acceptable/expected behaviour? Do we expect developers to semi-rely on get_type_hints(), but than still having to manually resolve forward references in PEP-585 generic aliases? That seems broken to me.
Thanks,
Niklas R.Niklas, can you show a brief example showing the issue you're running into? Is it just that list["Node"].__args__ is just ("Node",), not (ForwardRef("Node"),)? Or is it more complex?
NiklasRosenstein commented
on Jan 25, 2022 NiklasRosensteinmannequinMannequinGuido, sorry for the late response on this. I have a work around, but it involves passing along my own "context" from which to resolve strings on the go as they are encountered while decomposing the type hint.
NiklasRosenstein/python-databind@960da61
I'm using
typing.get_type_hints()expecting it to fully resolve all forward references, but that no longer happens with PEP-585 generics.I understand the documentation has been updated to reflect this behaviour, but it was an issue for me because it broke it's original API contract.
In addition, forward references encoded as string literals are handled by evaluating them in globals and locals namespaces.
Arguably the same has happened when
include_extraswas added (Annotated was now stripped from the returned resolved type hints by default), but that had an easy fix by simply wrapping it withinclude_extra=Truedepending on the Python version. The fix for the forward references in PEP-585 was not so trivial because we can't hook into whatget_type_hints()does when it encounters a string.I asked for a brief example that explains your issue to me. Instead you sent me links to production code and patches to it. Sorry, but that doesn't help me understand your problem. Is there really no way that you can write a little story that goes along the lines of
In our production code, we use the pattern
foo blah blah:
spam spam Ham eggs anchovies
buzz buzz blah get_type_hints buzz buzza lot. As you can see, the get_type_hints call fails
when we switch from "Ham eggs" to "ham eggs".NiklasRosenstein commented
on Jan 25, 2022 NiklasRosensteinmannequinMannequinYou're right, let me trim it down:
In production we use
get_type_hints()a lot, expecting it to resolve strings as forward references as per it's original API contract. However, PEP-585 generics parametrized with strings in Python 3.10 doesn't work like that (as the documentation already points out).get_type_hints()itself does not fail, subsequently broke our code because it was not built to expect strings inGenericAlias.__args__.What I ask myself is what motivated the decision to change the behaviour for PEP-585 generics in
get_type_hints()and not go the extra mile.When PEP-585 was discussed and implemented we did not expect people to care as much about runtime types as they did.
I already explained that making list['Node'] incorporate a ForwardRef instance is unrealistic (we'd have to reimplement ForwardRef in C first).
It might be possible to change get_type_hints() to recognize strings, and deprecate ForwardRef altogether. But I suspect that that would break something else, since ForwardRef is documented (I had intended it to remain an internal detail but somehow it got exposed, I don't recall why).
Please stop asking why the decision was made (it sounds rather passive-aggressive to me) and start explaining the problem you are having in a way that we can actually start thinking about a solution.
I have boiled down the original example to a slightly simpler one (dataclasses are a red herring):
>>> from typing import get_type_hints, List >>> class N: ... c1: list["N"] ... c2: List["N"] ... >>> N.__annotations__ {'c1': list['N'], 'c2': typing.List[ForwardRef('N')]} >>> get_type_hints(N) {'c1': list['N'], 'c2': typing.List[__main__.N]}
The key problem here is that the annotation list['N'] is not expanded to list[N]. What can we do to make get_type_hint() produce list[N] instead here?
NiklasRosenstein commented
on Jan 25, 2022 NiklasRosensteinmannequinMannequinIt was not my intention to sound passive agressive. Thanks for providing the context around the time PEP-585 was discussed.
Frankly, I believe I have explained the problem quite well. But I would like to propose a solution.
I can't judge in what circumstance a
strwould end up intyping._GenericAlias.__args__sincetyping._GenericAlias.__getitem__()converts strings totyping.ForwardRefusing_type_check()immediately.Assuming this is indeed a case to be taken into account, I would propose that
typing.get_type_hint()implements special treatment fortypes.GenericAliassuch that strings in__args__are treated as forward references, and keep the old behaviour fortyping._GenericAlias. Do you see any problems with that?Of course this would break cases that have come to strictly expect strings in PEP-585 generic
__args__since the release of Python 3.10 to stay strings, although I cannot come up with an example or think of a usecase myself.Ah, I see the issue. I stepped through get_type_hints() using pdb, and it does have a special case for when it encounters a string: it wraps it in a ForwardRef and proceeds from there:
Lines 1806 to 1807 in cef0a54
But list['N'] isn't a string, so it doesn't trigger this case. If you were to use "list[N]" instead, it works:
>>> from typing import get_type_hints >>> class N: ... c: "list[N]" ... >>> get_type_hints(N) {'c': list[__main__.N]} >>>
But I suppose you have a reason you (or your users) don't want to do that.
We could probably add a special case where it checks for types.GenericAlias and goes through __args__, replacing strings by ForwardRefs.
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?
Here's a patch that doesn't do it right but illustrates the point:
diff --git a/Lib/typing.py b/Lib/typing.py index 972b8ba24b..4616db60c3 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -1807,6 +1807,12 @@ def get_type_hints(obj, globalns=None, localns=None, include_extras=False): value = type(None) if isinstance(value, str): value = ForwardRef(value, is_argument=False, is_class=True) + elif isinstance(value, types.GenericAlias): + args = tuple( + ForwardRef(arg) if isinstance(arg, str) else args + for arg in value.__args__ + ) + value = value.__origin__[(*args,)] value = _eval_type(value, base_globals, base_locals) hints[name] = value
NiklasRosenstein commented
on Jan 25, 2022 NiklasRosensteinmannequinMannequinInteresting! Representing the entire type hint as a string is something I haven't thought about, but it makes sense that it works.
It is my understanding that
get_type_hint()already walks through the entire type hint recursively. If it weren't, it would not resolveList['N']toList[__main__.N]in the example below.>>> from typing import get_type_hints, Mapping, List >>> >>> class N: ... a: Mapping['str', list[List['N']]] ... >>> get_type_hints(N) {'a': typing.Mapping[str, list[typing.List[__main__.N]]]}
Upon closer inspection of the
typingcode, I can see that_eval_type()is doing that recursion. Applying the change your proposed in your previous message to that function seems to work at least in a trivial test case.diff --git a/Lib/typing.py b/Lib/typing.py index e3e098b1fc..ac56b545b4 100644 --- a/Lib/typing.py +++ b/Lib/typing.py @@ -331,6 +331,12 @@ def _eval_type(t, globalns, localns, recursive_guard=frozenset()): if isinstance(t, ForwardRef): return t._evaluate(globalns, localns, recursive_guard) if isinstance(t, (_GenericAlias, GenericAlias, types.UnionType)): + if isinstance(t, GenericAlias): + args = tuple( + ForwardRef(arg) if isinstance(arg, str) else arg + for arg in t.__args__ + ) + t = t.__origin__[(*args,)] ev_args = tuple(_eval_type(a, globalns, localns, recursive_guard) for a in t.__args__) if ev_args == t.__args__: return t
Testcase:
>>> from typing import get_type_hints, Mapping, List >>> class N: ... a: Mapping['str', List[list['N']]] ... >>> get_type_hints(N) {'a': typing.Mapping[str, typing.List[list[__main__.N]]]}
I believe that this would be enough, but then again I haven't yet had enough time to crack at other implications this might have.
How will it interact with from __future__ import annotations?
I've never used this future, but from my current, possibly limited, understanding it should have no side effects on how
get_type_hints()will evaluate fully stringified annotations (as you have already shown, a fully stringified type hint actually works fine with PEP-585 generics).And can we sell this as a bugfix for 3.10, or will this be a new feature in 3.11?
I will throw in my personal opinion that this could be a bugfix, but I'm obviously biased as being on the "experiencing end" of this behaviour we're trying to change.
NiklasRosenstein commented
on Jan 25, 2022 NiklasRosensteinmannequinMannequinI've started a pull request here: #30900
I think this is fixed now as the PR was merged but will leave closing this issue to @gvanrossum
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields: