GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-41341: Recursive evaluation of ForwardRef in get_type_hints by wyfo · Pull Request #21553 · python/cpython

Conversation

wyfo commented Jul 20, 2020 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Contributor

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.

https://bugs.python.org/issue41341

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          bpo-41341: Recursive evaluation of ForwardRef in get_type_hints

2389c25

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.

wyfo requested review from gvanrossum and ilevkivskyi as code owners

July 20, 2020 01:37

This comment has been minimized.

the-knights-who-say-ni added the CLA not signed label

Jul 20, 2020

bedevere-bot added the awaiting review label

Jul 20, 2020


          📜🤖 Added by blurb_it.

0a29c8b

the-knights-who-say-ni added CLA signed and removed CLA not signed labels

Jul 20, 2020

gvanrossum reviewed

Jul 21, 2020

View reviewed changes

gvanrossum left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isidentical This might help moving https://bugs.python.org/issue38605 forward (make from __future__ import annotations the default).

@ambv Assuming we get this to work, what would you think of backporting it to 3.9, even though beta5 is already released? On the one hand it's awfully late for such a change (and experience with the bpo issue above suggests it's not painful). On the other hand getting the same behavior for forward refs in 3.9 with from __future__ import annotations as it will have in 3.10 (regardless of that import) seems useful.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

Lib/test/test_typing.py

		{'a': tuple[T]})

		def test_double_forward(self):
		def foo(a: 'List[\'int\']'):

gvanrossum Jul 21, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was about to approve this PR when I realized that we should make this work with list -- I tried and it doesn't currently.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

wyfo Jul 22, 2020

Copy link

Copy Markdown

Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right concerning the fact that it doesn't work with list, but I've found that it's not related to this PR; actually, I think there is a significant defect in current PEP 585 implementation. In fact:

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

I will open an issue on bugs.python.org to discuss about this, because I think this PR is not the best place to do it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

johnandersen777 reacted with thumbs up emoji

All reactions

👍 1 reaction

gvanrossum Jul 22, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, you're right, thanks for the analysis. 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. Sorry for the hiccup. I will now approve your PR.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

isidentical commented Jul 22, 2020

Copy link

Copy Markdown

Member

@isidentical This might help moving https://bugs.python.org/issue38605 forward (make from future import annotations the default).

I had my own implementations, but I think I am going to rebase because it was kind of a workaround.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum approved these changes

Jul 22, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels

Jul 22, 2020

gvanrossum merged commit 653f420 into python:master

Jul 22, 2020

bedevere-bot commented Jul 22, 2020

Copy link

Copy Markdown

@gvanrossum: Please replace # with GH- in the commit message next time. Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the awaiting merge label

Jul 22, 2020

gvanrossum commented Jul 22, 2020

Copy link

Copy Markdown

Member

@ambv @pablogsal can I please have a decision on whether to backport this to 3.9? See my previous comment.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

pablogsal commented Jul 22, 2020

Copy link

Copy Markdown

Member

@ambv @pablogsal can I please have a decision on whether to backport this to 3.9? See my previous comment.

I think it makes sense, but being this 3.9 this is @ambv's decision

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ambv added the needs backport to 3.9 label

Jul 26, 2020

miss-islington commented Jul 26, 2020

Copy link

Copy Markdown

Contributor

Thanks @wyfo for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

Jul 26, 2020


          bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (pyth…

50ebde5

…onGH-21553)

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.
(cherry picked from commit 653f420)

Co-authored-by: wyfo <[email protected]>

bedevere-bot commented Jul 26, 2020

Copy link

Copy Markdown

GH-21629 is a backport of this pull request to the 3.9 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the needs backport to 3.9 label

Jul 26, 2020

miss-islington added a commit that referenced this pull request

Jul 26, 2020


          bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (GH-2…

41d1c04

…1553)

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.
(cherry picked from commit 653f420)

Co-authored-by: wyfo <[email protected]>

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request

Aug 4, 2020


          bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (pyth…

f766adc

…on#21553)

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request

Aug 20, 2020


          bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (pyth…

0af740d

…on#21553)

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.

xzy3 pushed a commit to xzy3/cpython that referenced this pull request

Oct 18, 2020


          bpo-41341: Recursive evaluation of ForwardRef in get_type_hints (pyth…

0dc2d89

…on#21553)

The issue raised by recursive evaluation is infinite recursion with
recursive types. In that case, only the first recursive ForwardRef is
evaluated.

johnandersen777 pushed a commit to johnandersen777/dffml that referenced this pull request

Jun 2, 2022


          util: python: is_forward_ref_dataclass: Helper to check for ForwardRe…

781f0c4

…f or str

Related: python/cpython#21553 (comment)
Signed-off-by: John Andersen <[email protected]>

johnandersen777 pushed a commit to johnandersen777/dffml that referenced this pull request

Jun 23, 2022


          util: python: is_forward_ref_dataclass: Helper to check for ForwardRe…

dbb7c17

…f or str

Related: python/cpython#21553 (comment)
Signed-off-by: John Andersen <[email protected]>

johnandersen777 pushed a commit to johnandersen777/dffml that referenced this pull request

Jun 23, 2022


          util: python: is_forward_ref_dataclass: Helper to check for ForwardRe…

1d2821f

…f or str

Related: python/cpython#21553 (comment)
Signed-off-by: John Andersen <[email protected]>

johnandersen777 pushed a commit to intel/dffml that referenced this pull request

Jul 7, 2022


          util: python: is_forward_ref_dataclass: Helper to check for ForwardRe…

fc2b052

…f or str

Related: python/cpython#21553 (comment)
Signed-off-by: John Andersen <[email protected]>

johnandersen777 pushed a commit to intel/dffml that referenced this pull request

Jul 7, 2022


          util: python: is_forward_ref_dataclass: Helper to check for ForwardRe…

ca4fb6b

…f or str

Related: python/cpython#21553 (comment)
Signed-off-by: John Andersen <[email protected]>

aronbierbaum mentioned this pull request

Aug 23, 2022

Update ForwardRef._evaluate for Python 3.11 python/typeshed#8600

Merged

hauntsaninja pushed a commit to python/typeshed that referenced this pull request

Aug 24, 2022


          Update ForwardRef._evaluate for Python 3.11 (#8600)

ef6baca

recursive_guard argument was added in python/cpython#21553

johnandersen777 pushed a commit to johnandersen777/dffml that referenced this pull request

Jan 5, 2023


          util: python: is_forward_ref_dataclass: Helper to check for ForwardRe…

8fcff78

…f or str

Related: python/cpython#21553 (comment)
Signed-off-by: John Andersen <[email protected]>

Reviewers

gvanrossum

gvanrossum approved these changes

ilevkivskyi

Awaiting requested review from ilevkivskyi

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

wyfo commented Jul 20, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

gvanrossum left a comment

Choose a reason for hiding this comment

Uh oh!

gvanrossum Jul 21, 2020

Choose a reason for hiding this comment

Uh oh!

wyfo Jul 22, 2020

Choose a reason for hiding this comment

Uh oh!

gvanrossum Jul 22, 2020

Choose a reason for hiding this comment

Uh oh!

isidentical commented Jul 22, 2020

Uh oh!

bedevere-bot commented Jul 22, 2020

Uh oh!

gvanrossum commented Jul 22, 2020

Uh oh!

pablogsal commented Jul 22, 2020

Uh oh!

miss-islington commented Jul 26, 2020

Uh oh!

bedevere-bot commented Jul 26, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

wyfo commented Jul 20, 2020 •

edited by bedevere-bot

Loading

Uh oh!

There was an error while loading. Please reload this page.