[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Guido van Rossum]

Guido van Rossum  added the comment:

Thank you Joseph Perez! Looking forward to more of your contributions.

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[miss-islington]

miss-islington  added the comment:


New changeset 41d1c04f73185c1238680142aa1a81f54f2bf4a4 by Miss Islington (bot) 
in branch '3.9':
bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (GH-21553)
https://github.com/python/cpython/commit/41d1c04f73185c1238680142aa1a81f54f2bf4a4


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 7.0 -> 8.0
pull_requests: +20770
pull_request: https://github.com/python/cpython/pull/21629

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Łukasz Langa]

Łukasz Langa  added the comment:

Given the previous behavior was clearly a bug and after looking at the PR, I 
think it should go into 3.9.0rc1.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Guido van Rossum]

Guido van Rossum  added the comment:

Łukasz, can I please have a decision on whether to backport this bugfix to 3.9?

See my comment about that:
https://github.com/python/cpython/pull/21553#pullrequestreview-452895735

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Guido van Rossum]

Guido van Rossum  added the comment:


New changeset 653f420b53a3aa87316cef59de8d3f5d9e11deb4 by wyfo in branch 
'master':
bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (#21553)
https://github.com/python/cpython/commit/653f420b53a3aa87316cef59de8d3f5d9e11deb4


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Joseph Perez]

Joseph Perez  added the comment:

Ok, I admit that I did not think about recursive type when proposing this "fix".
I've tried an implementation that just stop when recursion is encountered in a 
PR.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Joseph Perez]

Change by Joseph Perez :


--
keywords: +patch
pull_requests: +20699
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/21553

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue41341] Recursive evaluation of ForwardRef (and PEP 563)

[Joseph Perez]

New submission from Joseph Perez :

(This issue is already broached in https://bugs.python.org/issue38605, and a in 
some way in https://bugs.python.org/issue35834, but only as a secondary 
subject, that's why I've opened a ticket on this particular issue)

ForwardRef of ForwardRef are not currently evaluated by get_type_hints, only 
the first level is, as illustrated in these examples:
```python
from typing import ForwardRef, Optional, get_type_hints
def func(a: "Optional[\"int\"]"):
pass
assert get_type_hints(func)["a"] == Optional[ForwardRef("int")] 
# one would expect get_type_hints(func)["a"] == Optional[int] 
```
```python
from __future__ import annotations
from typing import ForwardRef, Optional, get_type_hints
def func(a: Optional["int"]):
pass
assert get_type_hints(func)["a"] == Optional[ForwardRef("int")]
# one would expect get_type_hints(func)["a"] == Optional[int] (which is the 
case without the import of __future__.annotations!)
```
On the one hand I find this behavior quite counter-intuitive; I rather think 
ForwardRef as kind of internal (and wonder why there is no leading underscore, 
like _GenericAlias where it's used) and I don't understand the purpose of 
exposing it as the result of the public API get_type_hints. By the way, if 
ForwardRef can be obtained by retrieving annotations without get_type_hints, 
stringified annotations (especially since PEP 563) make get_type_hints kind of 
mandatory, and thus make ForwardRef disappeared (only at the first level so …)

On the other hand, the second example show that adoptions of postponed 
annotations can change the result of get_type_hints; several libraries relying 
of get_type_hints could be broken.

An other issue raised here is that if these ForwardRef are not evaluated by 
get_type_hints, how will be done their evaluatation by the user? It would 
require to retrieve some globalns/localns — too bad, it's exactly what is doing 
get_type_hints. And if the ForwardRef is in a class field, the class 
globalns/localns will have to be kept somewhere while waiting to encounter 
these random ForwardRef; that's feasible, but really tedious.

Agreeing with Guido Von Rossum (https://bugs.python.org/msg370232), this 
behavior could be easily "fixed" in get_type_hints.
Actually, there would be only one line to change in ForwardRef._evaluate:
```python
# from
self.__forward_value__ = _type_check(
eval(self.__forward_code__, globalns, localns),
"Forward references must evaluate to types.",
is_argument=self.__forward_is_argument__)
# to
self.__forward_value__ = _eval_type(
_type_check(
eval(
self.__forward_code__, globalns, localns),
"Forward references must evaluate to types.",
is_argument=self.__forward_is_argument__,
),
globalns,
localns,
)

And if this fix could solve the "double ForwardRef" issue mentionned in 
https://bugs.python.org/issue38605, it would also resolve 
https://bugs.python.org/issue35834 in the same time, raising NameError in case 
of unknown ForwardRef with postponed annotation.

--
messages: 373960
nosy: BTaskaya, eric.smith, gvanrossum, joperez, levkivskyi, lukasz.langa, 
vstinner
priority: normal
severity: normal
status: open
title: Recursive evaluation of ForwardRef (and PEP 563)
type: behavior

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com