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.
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
inspect.signature includes bound argument for wrappers around bound methods #68486
Comments
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)
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. |
Reported by David Gibson here: https://bugzilla.redhat.com/show_bug.cgi?id=1201990 |
Thanks for reporting this, Petr! Nick, could you please take a look at the patch? |
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__. |
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. |
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) |
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. |
New changeset 42819b176d63 by Yury Selivanov in branch '3.4': New changeset d7e392c5c53a by Yury Selivanov in branch '3.5': New changeset ab46801ca359 by Yury Selivanov in branch 'default': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: