Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bpo-37806:Fix infinite recursion with typing.get_type_hints. #15493

Closed
wants to merge 2 commits into from
Closed

bpo-37806:Fix infinite recursion with typing.get_type_hints. #15493

wants to merge 2 commits into from

Conversation

hongweipeng
Copy link
Contributor

@hongweipeng hongweipeng commented Aug 25, 2019 •
edited by bedevere-bot
Loading

@gvanrossum gvanrossum removed their request for review August 25, 2019 20:36
@aeros aeros added needs backport to 3.8 only security fixes type-bug An unexpected behavior, bug, or error labels Aug 25, 2019
aeros
aeros approved these changes Aug 26, 2019
View reviewed changes
Copy link
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @hongweipeng.

I was able to confirm the bug on the latest commit to master and verify that the PR branch fixes it.

Master:
image

PR Branch:
image

aeros
aeros suggested changes Aug 26, 2019
View reviewed changes
Copy link
Contributor

@aeros aeros left a comment

Choose a reason for hiding this comment

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

A couple of minor suggestions for the Misc/NEWS entry:

Misc/NEWS.d/next/Library/2019-08-26-02-29-14.bpo-37806.wR83qu.rst Outdated
@@ -0,0 +1,2 @@
Fix infinite recursion with typing.get_type_hints.
Copy link
Contributor

@aeros aeros Aug 26, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This news entry can be condensed onto one line, the max character width for reST is 80.

Also, I would recommend adding a Sphinx role to link to the documentation for typing.get_type_hints. This can still fit on one line (exactly 80 chars in total).

Suggested change
Fix infinite recursion with typing.get_type_hints.
Fix infinite recursion from :func:`typing.get_type_hints`. Patch by hongweipeng.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok,thanks.

Misc/NEWS.d/next/Library/2019-08-26-02-29-14.bpo-37806.wR83qu.rst Outdated
@@ -0,0 +1,2 @@
Fix infinite recursion with typing.get_type_hints.
Patch by hongweipeng.
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above, this line can be condensed into the above line.

Suggested change
Patch by hongweipeng.

@ilevkivskyi
Copy link
Member

IIUC this is superseded by #15559 (which also looks better IMO), so I am closing this one.

@hongweipeng hongweipeng deleted the issue37806-get_type_hints branch August 31, 2019 15:10
@hongweipeng hongweipeng restored the issue37806-get_type_hints branch December 6, 2019 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aeros aeros aeros approved these changes

@ilevkivskyi ilevkivskyi Awaiting requested review from ilevkivskyi

Assignees
No one assigned
Labels
awaiting core review needs backport to 3.8 only security fixes type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@hongweipeng @ilevkivskyi @aeros @the-knights-who-say-ni @bedevere-bot