Issue 24298: inspect.signature includes bound argument for wrappers around bound methods (original) (raw)

Created on 2015-05-27 15:33 by petr.viktorin, last changed 2022-04-11 14:58 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
signature.patch yselivanov,2015-05-27 19:59 review
signature2.patch yselivanov,2015-05-27 21:52 review
Messages (8)
msg244178 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2015-05-27 15:33
When obtaining the signature of a bound method, inspect.signature, by default, omits the "self" argument to the method, since it is already specified in the bound method. However, if you create a wrapper around a bound method with functools.update_wrapper() or @functools.wraps, calling inspect.signature on the wrapper will return a signature which includes the "self" argument. Reproducer: import inspect import functools class Foo(object): def bar(self, testarg): pass f = Foo() @functools.wraps(f.bar) def baz(*args): f.bar(*args) assert inspect.signature(baz) == inspect.signature(f.bar) The program will fail with an assertion error. Examining inspect.signature(baz) shows: >>> print(inspect.signature(baz)) (self, testarg) >>> print(inspect.signature(f.bar)) (testarg) Looking at the code in inspect.py: The handling of bound methods appears at the top of inspect._signature_internal(). Since baz is not itself a bound method, it doesn't trigger this case. Instead inspect.unwrap is called, returning f.bar. inspect._signature_is_functionlike(f.bar) returns True, causing Signature.from_function to be called. Unlike the direct bound method case, this includes the bound method's "self" argument.
msg244179 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2015-05-27 15:33
Reported by David Gibson here: https://bugzilla.redhat.com/show_bug.cgi?id=1201990
msg244216 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-05-27 19:59
Thanks for reporting this, Petr! Nick, could you please take a look at the patch?
msg244228 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015-05-28 00:22
It occurs to me we're also bypassing the check that the unwrapped obj is a callable, so it probably makes sense to just recurse unconditionally after unwrapping the object, rather than only recursing for methods. That's also a little more future-proof, in case any further checks happen to be inserted ahead of the check for __wrapped__.
msg244234 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-05-28 01:04
> That's also a little more future-proof, in case any further checks happen to be inserted ahead of the check for __wrapped__. Well, we unwrap until we see a "__signature__" attribute (or we can't unwrap anymore). And right after unwrapping we try to return the __signature__; so inserting more checks before unwrapping with unconditional recursion after it won't be so safe.
msg244238 - (view) Author: Alyssa Coghlan (ncoghlan) * (Python committer) Date: 2015-05-28 01:25
I'm OK with the patch as is, but I'm definitely concerned about the maintainability of inspect.signature in general. I'm trying to decide if a block comment covering the order of calling protocols that we check, and where we potentially recurse, would be a help (by providing a map of the function for the benefit of future maintainers) or a hindrance (by providing the opportunity for that map to get out of sync)
msg244241 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-05-28 01:37
> I'm OK with the patch as is, but I'm definitely concerned about the maintainability of inspect.signature in general. I agree, _signature_from_callable is getting quite complex. The good news is that we have a very good test coverage for signatures. As for a big block comment -- it might be useful. OTOH the code of _signature_from_callable is mostly if..else statements with calls to helpers and recursion.
msg244243 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-05-28 02:01
New changeset 42819b176d63 by Yury Selivanov in branch '3.4': Issue 24298: Fix signature() to properly unwrap wrappers around bound methods https://hg.python.org/cpython/rev/42819b176d63 New changeset d7e392c5c53a by Yury Selivanov in branch '3.5': Issue 24298: Fix signature() to properly unwrap wrappers around bound methods https://hg.python.org/cpython/rev/d7e392c5c53a New changeset ab46801ca359 by Yury Selivanov in branch 'default': Issue 24298: Fix signature() to properly unwrap wrappers around bound methods https://hg.python.org/cpython/rev/ab46801ca359
History
Date User Action Args
2022-04-11 14:58:17 admin set github: 68486
2015-05-28 02:02:36 yselivanov set status: open -> closedresolution: fixedstage: patch review -> resolved
2015-05-28 02:01:32 python-dev set nosy: + python-devmessages: +
2015-05-28 01:37:44 yselivanov set messages: +
2015-05-28 01:25:07 ncoghlan set messages: +
2015-05-28 01:04:03 yselivanov set messages: +
2015-05-28 00:22:43 ncoghlan set messages: +
2015-05-27 21:52:14 yselivanov set files: + signature2.patch
2015-05-27 19:59:27 yselivanov set files: + signature.patchversions: + Python 3.6messages: + assignee: yselivanovkeywords: + patchstage: patch review
2015-05-27 16:34:40 yselivanov set nosy: + ncoghlan
2015-05-27 16:34:25 yselivanov set nosy: + yselivanov
2015-05-27 15:33:45 petr.viktorin set messages: +
2015-05-27 15:33:36 petr.viktorin create