msg244178 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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) *  |
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)  |
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 |
|
|