bpo-37838: get_type_hints for wrapped functions with forward reference by benedwards14 · Pull Request #17126

bpo-37838: get_type_hints for wrapped functions with forward reference by benedwards14 · Pull Request #17126 · python/cpython

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.

		def dec(func):
		def wrapper(args, *kwargs):
		return func(args, *kwargs)
		return wraps(func)(wrapper) No newline at end of file

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:

	return wraps(func)(wrapper)
	return wraps(func)(wrapper)


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

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?

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


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

Choose a reason for hiding this comment

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

See above:

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

		self.assertEqual(gth(G), {'lst': ClassVar[List[T]]})

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

Choose a reason for hiding this comment

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

See above:

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

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`.

Benjamin Edwards and others added 2 commits

November 21, 2019 03:37

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:


		gth = get_type_hints

		class ForRefExample():

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:

	class ForRefExample():

	class ForRefExample:

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!

Co-Authored-By: Brandt Bucher <brandtbucher@gmail.com>

Choose a reason for hiding this comment

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

Looks good, thanks!

(Added backport labels since this is a bugfix.)

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

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

Nov 21, 2019

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

Nov 21, 2019

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

Dec 5, 2019

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

Jan 31, 2020