inspect.signature includes bound argument for wrappers around bound methods

[encukou]

BPO	24298
Nosy	@ncoghlan, @encukou, @1st1
Files	signature.patch signature2.patch

^{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:

assignee = 'https://github.com/1st1'
closed_at = <Date 2015-05-28.02:02:36.839>
created_at = <Date 2015-05-27.15:33:36.637>
labels = []
title = 'inspect.signature includes bound argument for wrappers around bound methods'
updated_at = <Date 2015-05-28.02:02:36.839>
user = 'https://github.com/encukou'

bugs.python.org fields:

activity = <Date 2015-05-28.02:02:36.839>
actor = 'yselivanov'
assignee = 'yselivanov'
closed = True
closed_date = <Date 2015-05-28.02:02:36.839>
closer = 'yselivanov'
components = []
creation = <Date 2015-05-27.15:33:36.637>
creator = 'petr.viktorin'
dependencies = []
files = ['39524', '39526']
hgrepos = []
issue_num = 24298
keywords = ['patch']
message_count = 8.0
messages = ['244178', '244179', '244216', '244228', '244234', '244238', '244241', '244243']
nosy_count = 4.0
nosy_names = ['ncoghlan', 'petr.viktorin', 'python-dev', 'yselivanov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue24298'
versions = ['Python 3.4', 'Python 3.5', 'Python 3.6']

[encukou]

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.

[encukou]

Reported by David Gibson here: https://bugzilla.redhat.com/show_bug.cgi?id=1201990

[1st1]

Thanks for reporting this, Petr!

Nick, could you please take a look at the patch?

[ncoghlan]

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

[1st1]

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.

[ncoghlan]

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)

[1st1]

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.

[python-dev]

New changeset 42819b176d63 by Yury Selivanov in branch '3.4':
bpo-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':
bpo-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':
bpo-24298: Fix signature() to properly unwrap wrappers around bound methods
https://hg.python.org/cpython/rev/ab46801ca359