classification
Title: inspect.getfullargspec should use __signature__
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: 20189 Superseder:
Assigned To: yselivanov Nosy List: Claudiu.Popa, Yury.Selivanov, larry, michael.foord, ncoghlan, python-dev, terry.reedy, yselivanov
Priority: normal Keywords: needs review, patch

Created on 2013-03-19 17:31 by michael.foord, last changed 2014-01-29 16:54 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
getargsspec_01.patch yselivanov, 2014-01-11 22:24 review
getargsspec_02.patch yselivanov, 2014-01-20 16:02
getargsspec_03.patch yselivanov, 2014-01-27 19:34
getargsspec_04.patch yselivanov, 2014-01-28 18:02 review
Messages (31)
msg184646 - (view) Author: Michael Foord (michael.foord) * (Python committer) Date: 2013-03-19 17:31
inspect.getfullargspec (and potentially getargspec?) could *use* function.__signature__ if set. This allows functions that lie about their signature with the new introspection tool (Signature) to still work with older code.
msg207919 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-11 22:24
Please consider the attached patch (getargsspec_01.patch).

It modifies 'getargspec' and 'getfullargspec' to use the 'inspect.signature' API. The entire test suite passes just fine.

This also will address issue #16490.

I can also update the docs, if it's necessary.
msg208369 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-17 22:46
Can somebody review the patch? I'd be cool if this lands in 3.4.
msg208416 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-01-18 20:26
While I plan to change Idle code to use signature directly, instead of getfullargspec, I agree that changing inspect also, and in 3.4, is a good idea. It may, however, affect tests other than Idle's, if there are any other stdlib consumers. I will try to look at the patch after changing Idle.
msg208437 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-01-19 03:54
I upgraded the description to a "should". Argument Clinic and other changes in Python 3.4 greatly improve introspection support for various aspects of the runtime and standard library (for example, issue 20223 will handle the new functools.partialmethod support). By also making those enhancements available via getfullargpsec, we should significantly increase the amount of existing code which benefits automatically for those enhancements, as it will no longer require explicit migration to the new PEP 362 APIs at the application/library level.

Reviewing the patch now.
msg208440 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-19 03:59
If we modify inspect.getfullargspec, shouldn't we modify inspect.getargspec too?  "deprecated" doesn't mean "unsupported", it means "not recommended for new code, please stop using it".
msg208442 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-19 04:28
Larry, 
getargspec uses getfullargspec, so it's covered.
msg208443 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-01-19 04:36
getargspec() is just a thin wrapper around getfullargspec(), so we get that for free.

Similarly, getcallargs() is also implicitly updated to full PEP 362 support by changing the implementation of getfullargspec().

The other two standard library consumers appear to be IDLE (as Terry noted above) and xmlrpc.server.ServerHTMLDoc.docroutine (the self-documenting XML-RPC server API, which sadly appears to be lacking test cases).

Reviewing those cases (especially the XML-RPC one) have convinced me we need a backwards compatibility fallback in the updated getfullargspec implementation. Specifically, if the new API throws ValueError (because it can't find a way to build a coherent signature object), we should fall back to the old approach. Otherwise we run the risk of introducing unexpected exceptions into introspection code.

That is, the new call to signature in getfullargspec should look something like:

    try:
        sig = signature(func)
    except ValueError:
        if not hasattr(func, "__code__"):
            raise # The legacy fallback doesn't support this input type
        # The callable signature appears to be incoherent, so we fall
        # back to the legacy implementation to preserve compatibility
        args, varargs, kwonlyargs, varkw = _getfullargs(func.__code__)
        return FullArgSpec(args, varargs, varkw, func.__defaults__,
                kwonlyargs, func.__kwdefaults__, func.__annotations__)

I suspect this may actually be a better solution for IDLE rather than updating it to call inspect.signature directly (since IDLE probably *wants* the more graceful fallback to the old behaviour). 

The way I would document all this in What's New is to update the current argument clinic section to also explain that we've taken advantage of PEP 362 to improve introspection in multiple areas, including for code that is using the introspection APIs that predate PEP 362.
msg208470 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-01-19 10:33
The relevant code in CallTips.py is
  argspec = ""
  if hasattr(ob, '__call__'):
...
    if isinstance(fob, (types.FunctionType, types.MethodType)):
      argspec = inspect.formatargspec(*inspect.getfullargspec(fob))

So I want to broaden the second condition (or remove it), but if it returns something .formatargspec cannot digest, the outer call will have to be conditioned. My impression is that str(signature object) returns pretty much what .formatargspec does, but I have not experimented yet.

My #20122 patch moves currently commented out tests in CallTips.py that use the above to test_calltips.py. I will commit as soon as I test on 3.4. Until then, getfullargspec is not being used in the Idle test suite.
msg208471 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-19 10:44
I just want to mention, while we're all thinking about this stuff: I plan to enhance the Signature object to reflect "optional groups".  Right now Signature objects cannot express parameters that are optional but don't have a published default.  (e.g. a string parameter for a builtin having a default of NULL).  A lot of these are being converted using optional groups, and I want to make it at least possible for user code to introspect those functions and understand that those are optional parameters.

My original plan was to add a "group" member, containing an arbitrary identifier of the "group" this parameter belongs to.  I'm not sure that's sufficient; I may also need a "parent_group" parameter, or something like that, to represent the group that this group is nested in.  I'm still thinking about it.  But I'm hoping to get to this in the next two or three days.
msg208475 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-01-19 12:15
Sounds reasonable - we'll need that to convert range() and slice()
anyway (I totally failed at finding time to look at the builtins this
weekend, but I'd still like to handle those before the 3rd beta).

An alternative would be to actually add "ParameterGroup" as a new
class that contains parameters (assuming tuple isn't sufficient), so
the structure of the parameters more directly matches the underlying
signature.
msg208484 - (view) Author: Yury Selivanov (Yury.Selivanov) * Date: 2014-01-19 17:16
Larry,  

I saw your message on the tracker regarding adding support for parameters groups to the signature object. Would you mind if I join the discussion with my ideas of how this feature might be implemented? 

Yury

On Sunday, January 19, 2014 at 5:44 AM, Larry Hastings wrote:

> 
> Larry Hastings added the comment:
> 
> I just want to mention, while we're all thinking about this stuff: I plan to enhance the Signature object to reflect "optional groups". Right now Signature objects cannot express parameters that are optional but don't have a published default. (e.g. a string parameter for a builtin having a default of NULL). A lot of these are being converted using optional groups, and I want to make it at least possible for user code to introspect those functions and understand that those are optional parameters.
> 
> My original plan was to add a "group" member, containing an arbitrary identifier of the "group" this parameter belongs to. I'm not sure that's sufficient; I may also need a "parent_group" parameter, or something like that, to represent the group that this group is nested in. I'm still thinking about it. But I'm hoping to get to this in the next two or three days.
> 
> ----------
> 
> _______________________________________
> Python tracker <report@bugs.python.org (mailto:report@bugs.python.org)>
> <http://bugs.python.org/issue17481>
> _______________________________________
msg208495 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-01-19 20:07
Yuri, I am sure your ideas for enhancing signature objects would be welcome. Either a new issue or a thread on pydev or python-ideas lists would be best.

When responding by email, please snip the quotation and footer, except possibly a line of the quoted message.
msg208496 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-19 20:11
Terry,

Thanks. 

> When responding by email, please snip the quotation and footer, except possibly a line of the quoted message.

My email client played an evil (and a bit embarrassing) trick with me, showing Larry's name without an actual email address, which was "report@bugs.python.org". Won't happen again.
msg208505 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-19 22:18
> Otherwise we run the risk of introducing unexpected exceptions into introspection code.

That's a good catch. I'll make a new patch, keeping the old implementation of getfullargsspec intact, and falling back to it if no signature can be found.
msg208506 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-19 22:22
Yury: fire away, either here or in a new issue as is best.  (Sorry, brain mildly fried, not sure what would be best issue-tracker hygiene.)
msg208509 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-19 22:56
>> Otherwise we run the risk of introducing unexpected exceptions into introspection code.

> That's a good catch. I'll make a new patch, keeping the old implementation of getfullargsspec intact, and falling back to it if no signature can be found.

Nick, while I was working on the second patch (writing a new unittest for it specifically), I realized, that it's not that easy to make the old version of "getfullargsspec" to spit out any exception that it doesn't currently do with the proposed 'getargsspec_01.patch'.

See, the old "getfullargsspec" does the following:

1. Check if the passed object is a function, with 'inspect.isfunction'. If not - throw a TypeError.  That behaviour is duplicated in the patch, so we are safe here.

2. Call on the object's __code__ '_getfullargs', which validates that the passed code object is a valid code object, and simply returns its attributes rearranged a bit.

Now, to have any exception in (2), we need: either a broken __code__ object, or something that is an instance of "types.FunctionType" (hence, defined in python) but doesn't have the "__code__" attribute. And that's kind of hard to achieve.
msg208517 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-20 02:45
> Yury: fire away, either here or in a new issue as is best.  (Sorry, brain mildly fried, not sure what would be best issue-tracker hygiene.)

Larry, I ended up with a relatively big core dump of my thoughts, so I decided to post it on python-dev. Let's discuss it there, and we can create an issue here once we decide something.
msg208535 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-01-20 12:10
Ah, I had indeed missed the fact that getfullargspec() was still calling isfunction(). In that case, is the patch currently actually buying us anything much beyond handling __signature__ attributes? Most of the new types that inspect.signature() supports will still fail that preliminary check, so code will need to use the new API explicitly in order to benefit from it.

By contrast, if we remove the check, then there's a wider range of exceptions that may be thrown, but also a much wider variety of inputs supported.

Instead of keeping the check, we could just unconditionally convert exceptions from the signature call to a TypeError in order to maintain compatibility with the old external behaviour.
msg208547 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-20 16:02
> Instead of keeping the check, we could just unconditionally convert exceptions from the signature call to a TypeError in order to maintain compatibility with the old external behaviour.

Agreed. See the new patch (getargsspec_02.patch)

Unfortunately, we have to keep the old 'ismethod' check in place for backwards compatibility purposes.

Larry,

The attached patch contains one failing unit-test: 'inspect.signature' returns 'None' for '_testapi.docstring_no_signature'. It should instead raise a ValueError.
msg208549 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-20 16:57
Larry,

I created a separate issue for that: http://bugs.python.org/issue20313
msg208971 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-23 18:07
Larry, Nick,

So what's the resolution on this one? Do I have a green light?
msg209025 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-23 23:56
My solution for pydoc was to call isroutine() instead of isfunction(), and yes handle it throwing an exception.

(I just checked, and I wasn't catching TypeError, only ValueError.  I'll fix that in my next patch for #20189.)
msg209077 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2014-01-24 14:03
Another case of "don't land it until Larry has dealt with the builtins".

The patch itself looks fine, though :)
msg209082 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-24 14:25
The patch from #20189 has landed.
msg209141 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-25 03:56
There's a major difference between getfullargspec/getargspec and inspect.signature: getfullargspec shows you the "self" parameter for bound methods, and inspect.signature does not.

>>> class C:
...    def foo(self, a):  pass
... 
>>> c = C()
>>>
>>> import inspect
>>> str(inspect.signature(c.foo))
'(a)'
>>> inspect.getfullargspec(c.foo)
FullArgSpec(args=['self', 'a'], varargs=None, varkw=None, defaults=None, kwonlyargs=[], kwonlydefaults=None, annotations={})
>>> inspect.getargspec(c.foo)
ArgSpec(args=['self', 'a'], varargs=None, keywords=None, defaults=None)

This is why help() (currently) shows bound parameters for methods implemented in Python, but doesn't show them for methods implemented in C.  pydoc uses inspect.getfullargspec for pure Python functions and inspect.signature for builtins.
msg209469 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-27 19:34
> There's a major difference between getfullargspec/getargspec and inspect.signature: getfullargspec shows you the "self" parameter for bound methods, and inspect.signature does not.

Larry, yes, that's correct. The attached patch simulates this behaviour, with:

    if ismethod(func):
        func = func.__func__

I'm attaching 'getargsspec_03.patch', as the previous one (02) was a bit crippled.
msg209569 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-28 18:02
Larry and Nick,

Please review the final patch (getargsspec_04.patch). I'd like to commit it tomorrow.

This one is the last one (hopefully), and it supports builtin methods properly -- i.e. works around 'self' parameter correctly. To do that, I check if the '__text_signature__' starts with "($", etc.
msg209655 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-29 16:45
New changeset 6d1e8162e855 by Yury Selivanov in branch 'default':
inspect.getfullargspec: Use inspect.signature API behind the scenes #17481
http://hg.python.org/cpython/rev/6d1e8162e855
msg209656 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2014-01-29 16:46
Committed.
Thanks for the reviews!
msg209657 - (view) Author: Roundup Robot (python-dev) Date: 2014-01-29 16:54
New changeset 0fa2750c7241 by Yury Selivanov in branch 'default':
inspect.test.getfullargspec: Add a unittest to ensure correct annotations
http://hg.python.org/cpython/rev/0fa2750c7241
History
Date User Action Args
2014-01-29 20:55:18yselivanovlinkissue8639 dependencies
2014-01-29 16:54:26python-devsetmessages: + msg209657
2014-01-29 16:46:22yselivanovsetstatus: open -> closed
resolution: fixed
2014-01-29 16:46:15yselivanovsetmessages: + msg209656
2014-01-29 16:45:48python-devsetnosy: + python-dev
messages: + msg209655
2014-01-28 18:22:04yselivanovsetkeywords: + needs review
2014-01-28 18:02:10yselivanovsetfiles: + getargsspec_04.patch
assignee: yselivanov
messages: + msg209569

stage: patch review
2014-01-27 19:34:29yselivanovsetfiles: + getargsspec_03.patch

messages: + msg209469
2014-01-25 03:56:05larrysetmessages: + msg209141
2014-01-24 14:25:39larrysetmessages: + msg209082
2014-01-24 14:03:29ncoghlansetdependencies: + inspect.Signature doesn't recognize all builtin types
messages: + msg209077
2014-01-23 23:56:37larrysetmessages: + msg209025
2014-01-23 18:07:46yselivanovsetmessages: + msg208971
2014-01-20 16:57:15yselivanovsetmessages: + msg208549
2014-01-20 16:02:06yselivanovsetfiles: + getargsspec_02.patch

messages: + msg208547
2014-01-20 12:10:19ncoghlansetmessages: + msg208535
2014-01-20 02:45:50yselivanovsetmessages: + msg208517
2014-01-19 22:56:42yselivanovsetmessages: + msg208509
2014-01-19 22:22:52larrysetmessages: + msg208506
2014-01-19 22:18:39yselivanovsetmessages: + msg208505
2014-01-19 20:11:36yselivanovsetmessages: + msg208496
2014-01-19 20:07:36terry.reedysetmessages: + msg208495
2014-01-19 17:16:57Yury.Selivanovsetnosy: + Yury.Selivanov
messages: + msg208484
2014-01-19 12:15:03ncoghlansetmessages: + msg208475
2014-01-19 10:44:43larrysetmessages: + msg208471
2014-01-19 10:33:46terry.reedysetmessages: + msg208470
2014-01-19 04:36:06ncoghlansetmessages: + msg208443
2014-01-19 04:28:28yselivanovsetmessages: + msg208442
2014-01-19 03:59:02larrysetmessages: + msg208440
2014-01-19 03:54:37ncoghlansetmessages: + msg208437
title: inspect.getfullargspec could use __signature__ -> inspect.getfullargspec should use __signature__
2014-01-18 20:26:24terry.reedysetmessages: + msg208416
2014-01-18 14:35:13yselivanovsetnosy: + terry.reedy
2014-01-17 22:46:20yselivanovsetnosy: + larry
messages: + msg208369
2014-01-11 23:38:06Claudiu.Popasetnosy: + Claudiu.Popa
2014-01-11 22:24:56yselivanovsetfiles: + getargsspec_01.patch

nosy: + yselivanov
messages: + msg207919

keywords: + patch
2013-03-19 17:31:32michael.foordcreate