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-37838: get_type_hints for wrapped functions with forward reference #17126

Merged
merged 5 commits into from
Nov 21, 2019
Merged

bpo-37838: get_type_hints for wrapped functions with forward reference #17126

merged 5 commits into from
Nov 21, 2019

Conversation

benedwards14
Copy link
Contributor

@benedwards14 benedwards14 commented Nov 12, 2019 •
edited by bedevere-bot
Loading

brandtbucher
brandtbucher requested changes Nov 12, 2019
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

This looks like a good change to me! Just a few comments below to fix CI and clean things up.

Lib/test/ann_module.py Outdated
def dec(func):
def wrapper(*args, **kwargs):
return func(*args, **kwargs)
return wraps(func)(wrapper)
Copy link
Member

Choose a reason for hiding this comment

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

I think the tests are failing because of this missing newline:

Suggested change
return wraps(func)(wrapper)
return wraps(func)(wrapper)

Lib/test/test_typing.py Outdated
@@ -2778,6 +2778,15 @@ class HasForeignBaseClass(mod_generics_cache.A):

gth = get_type_hints

class ForRefExample():
@ann_module.dec
def func(a: 'ForRefExample'):
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but maybe just name this argument self for cleanliness?

Suggested change
def func(a: 'ForRefExample'):
def func(self: 'ForRefExample'):

Lib/test/test_typing.py Outdated

@ann_module.dec
@ann_module.dec
def nested(a: 'ForRefExample'):
Copy link
Member

Choose a reason for hiding this comment

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

See above:

Suggested change
def nested(a: 'ForRefExample'):
def nested(self: 'ForRefExample'):

Lib/test/test_typing.py Outdated
@@ -2876,6 +2885,10 @@ def test_get_type_hints_ClassVar(self):
'x': ClassVar[Optional[B]]})
self.assertEqual(gth(G), {'lst': ClassVar[List[T]]})

def test_get_type_hints_wrapped_decoratored_func(self):
expects = {'a': ForRefExample}
Copy link
Member

Choose a reason for hiding this comment

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

See above:

Suggested change
expects = {'a': ForRefExample}
expects = {'self': ForRefExample}

@brandtbucher brandtbucher added the type-bug An unexpected behavior, bug, or error label Nov 12, 2019
@brandtbucher
Copy link
Member

Thanks for the PR @benedwards14!

This one should also have a NEWS entry. Just something simple, like:

:meth:`typing.get_type_hints` properly handles functions decorated with :meth:`functors.wraps`.

@benedwards14
Copy link
Contributor Author

benedwards14 commented Nov 21, 2019 •
edited
Loading

Hey @brandtbucher, thanks for the review. Here are the markups.

brandtbucher
brandtbucher requested changes Nov 21, 2019
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of remaining style nitpicks:

Lib/test/test_typing.py Outdated
@@ -2778,6 +2778,15 @@ class HasForeignBaseClass(mod_generics_cache.A):

gth = get_type_hints

class ForRefExample():
Copy link
Member

Choose a reason for hiding this comment

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

No parentheses here, and 2 blank lines between top-level definitions:

Suggested change
class ForRefExample():
class ForRefExample:

brandtbucher
brandtbucher requested changes Nov 21, 2019
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Sorry, one last typo I spotted. Then good!

@benedwards14 @brandtbucher
Update Misc/NEWS.d/next/Library/2019-11-21-11-39-17.bpo-37838.lRFcEC.rst
f7815a2 
Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com>
@brandtbucher
Copy link
Member

@ilevkivskyi

ilevkivskyi
ilevkivskyi approved these changes Nov 21, 2019
View reviewed changes
Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@ilevkivskyi
Copy link
Member

(Added backport labels since this is a bugfix.)

@ilevkivskyi ilevkivskyi merged commit 0aca3a3 into python:master Nov 21, 2019
@miss-islington
Copy link
Contributor

Thanks @benedwards14 for the PR, and @ilevkivskyi for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-17324 is a backport of this pull request to the 3.8 branch.

@sthagen sthagen mentioned this pull request Nov 21, 2019
Merged
@bedevere-bot
Copy link

GH-17325 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 21, 2019
@benedwards14 @miss-islington
bpo-37838: get_type_hints for wrapped functions with forward reference (
324ba4b 
pythonGH-17126)

https://bugs.python.org/issue37838
(cherry picked from commit 0aca3a3)

Co-authored-by: benedwards14 <53377856+benedwards14@users.noreply.github.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 21, 2019
@benedwards14 @miss-islington
bpo-37838: get_type_hints for wrapped functions with forward reference (
20a0c7e 
pythonGH-17126)

https://bugs.python.org/issue37838
(cherry picked from commit 0aca3a3)

Co-authored-by: benedwards14 <53377856+benedwards14@users.noreply.github.com>
miss-islington added a commit that referenced this pull request Nov 21, 2019
@miss-islington @benedwards14
bpo-37838: get_type_hints for wrapped functions with forward reference (
30e5bd8 
GH-17126)

https://bugs.python.org/issue37838
(cherry picked from commit 0aca3a3)

Co-authored-by: benedwards14 <53377856+benedwards14@users.noreply.github.com>
miss-islington added a commit that referenced this pull request Nov 21, 2019
@miss-islington @benedwards14
bpo-37838: get_type_hints for wrapped functions with forward reference (
9458c5c 
GH-17126)

https://bugs.python.org/issue37838
(cherry picked from commit 0aca3a3)

Co-authored-by: benedwards14 <53377856+benedwards14@users.noreply.github.com>
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@benedwards14
bpo-37838: get_type_hints for wrapped functions with forward reference (
d582608 
pythonGH-17126)

https://bugs.python.org/issue37838
shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
@benedwards14 @shihai1991
bpo-37838: get_type_hints for wrapped functions with forward reference (
6d41fe9 
pythonGH-17126)

https://bugs.python.org/issue37838
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandtbucher brandtbucher brandtbucher requested changes

@ilevkivskyi ilevkivskyi ilevkivskyi approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

Assignees
No one assigned
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@benedwards14 @brandtbucher @ilevkivskyi @miss-islington @bedevere-bot @the-knights-who-say-ni