classification
Title: Changes in the inspect module for PEP 570
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, hroncok, lukasz.langa, ncoghlan, pablogsal, serhiy.storchaka, steve.dower, vstinner, zzzeek
Priority: normal Keywords: 3.7regression, patch

Created on 2019-04-29 12:13 by pablogsal, last changed 2019-05-22 12:21 by ncoghlan. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 13016 merged pablogsal, 2019-04-29 22:31
PR 13245 merged pablogsal, 2019-05-11 15:19
Messages (30)
msg341070 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-04-29 12:13
This issue is to discuss how to handle the changes in the inspect module for PEP 570. In particular, how to handle:

* getfullargspec
* formatargspec

for positional-only parameters.
msg341075 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-29 13:02
Context: see https://github.com/python/cpython/pull/12701 discussion.
msg341081 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-04-29 13:40
Marking as a release blocker and adding RM. The 3.8 branch now contains breaking API changes to these functions that we have to resolve before the next release (or revert while deciding how to handle the change) - PR 12701 has been merged.
msg341082 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-04-29 13:43
My bad, there were PR changes that GitHub didn't show me. As you were
msg341083 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-04-29 14:02
Nope, I was right the first time. The FullArgSpec tulle has changed indexes, and formatargspec has additional (undocumented) arguments.

Since formatargspec is deprecated, it should probably just not change.

The FullArgSpec tuple might have to become a concrete class implementation so that it can have positional-only args as a named attribute but not part of unpacking.

As for where the new information *can* go, I suspect the Signature object is the only place. Though that has limitations, and for C APIs in particular much of my code only uses the more reliable getfullargspec.
msg341098 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-04-29 17:02
Steve's right that we can't change the indexes on those functions for pre-existing values, so appending to the end is the only real option in that case. I still think they should both be deprecated due to how limiting their APIs are.

Obviously signature() can be updated cleanly.
msg341101 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-29 17:10
IMHO getargs(), getargspec(), getfullargspec() should be deprecated in favor of signature() which is future-proof. But Nick Coghlan undeprecated these functions in 2017 https://bugs.python.org/issue20438 whereas these were deprecated since 2015. I didn't understand the subtle issues. That's why I asked Pablo to open a separated issue, to get Nick and others involved to get the context.

My proposal was to raise an exception if the input function has positonal arguments. There is good way to fix these legacy functions, their design cannot be extended, whereas signature() is fine.
msg341102 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-04-29 17:32
> My proposal was to raise an exception if the input function has positonal arguments

Counter-proposal - just report them as regular position-or-name arguments.

Users of inspect are most likely to be IDEs or editors getting information to present to users. And in my experience, most just ignore all exceptions because bugs cause some function objects to raise errors when trying to inspect them. Which means a new exception will just result in a worse experience for users and no indication of what's wrong.

On the other hand, if there is no change made, they'll just report a normal looking signature until they migrate to using updated Signature objects. Anyone using current Signature objects should similarly get the argument count right, even if they totally ignore the positional-only count. Since one of the arguments for supporting positional-only parameters is that they're for parameters that you'd never consider passing by name, but these tools still need to show a name, very few people should ever be tripped up by them being misreported. And when they do they'll be tripped up very quickly.

Basically, this is not a scenario that *requires* existing users to be broken. So we should avoid breaking them at all costs.
msg341104 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-04-29 17:49
If I remember correctly the un-deprecation was because 2/3 code could only use getfullargspec() to get all relevant details. Since we're so close to being done w/ Python 2 I think this might be the last change to make to them, document their deprecation, tack this info on to the end of the tuple, and then start raising DeprecationWarning in 3.9.
msg341115 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-04-29 19:43
I'm with Steve on this one. Report positional-only args as regular arguments in the old functions.
msg341124 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-04-29 22:35
PR 13016 adds the positional-only arguments together with the regular arguments as suggested by Steve and Łukasz and deprecates getfullargspec() in favour of inspect.signature.
msg341128 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-04-30 01:01
New changeset d5d2b4546939b98244708e5bb0cfccd55b99d244 by Pablo Galindo in branch 'master':
bpo-36751: Deprecate getfullargspec and report positional-only args as regular args (GH-13016)
https://github.com/python/cpython/commit/d5d2b4546939b98244708e5bb0cfccd55b99d244
msg341130 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-04-30 01:02
PR 13016 is merged, and I will remove the release blocker, but I will leave it open for now until we can check if there is consensus on the status and future of the inspect module after PEP 570 :)
msg342156 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-11 04:24
This PR needs to be reverted - we previously deprecated this API, but then undeprecated it again in #27172, as the consequence of deprecating it was projects copying & pasting the getfullargspec() implementation into their own code, not switching to inspect.Signature.
msg342157 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-11 04:32
And no, the undeprecation wasn't because of Python 2 (Py2 doesn't have getfullargspec() - it's a Py3 only API).

The undeprecation was because there are a lot of 3rd party projects for whom the getfullargspec() representation is good enough, and switching to inspect.Signature instead requires a significant rewrite vs just wrapping inspect.Signature to produce getfullargspec() style output.

So getfullargspec() doesn't need to change at all for PEP 570 and should instead keep handling positional only arguments the same way it has since it was switched over to being based on inspect.Signature: reporting them the same way as positional-or-keyword parameters.
msg342158 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-11 04:35
Also note #32190 regarding changing the way these APIs are documented, without introducing any programmatic deprecation warnings.
msg342197 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-05-11 15:23
Opened PR 13245 to un-deprecate getfullargspec.
msg342295 - (view) Author: Miro Hrončok (hroncok) * Date: 2019-05-13 09:22
Just a quick idea: What if a warning happened iff positional only argument are there, but not with functions that don't use those? Might be a different kind of warning.
msg342296 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-13 09:28
> And no, the undeprecation wasn't because of Python 2 (Py2 doesn't have getfullargspec() - it's a Py3 only API).

Python 3.8.0 is scheduled for: "3.8.0 final: Monday, 2019-10-21"

Something like 2 months before 2.7 end of support: 2019-12-31.

Does the "because of Python 2" still stand for Python 3.8?

A deprecating warning doesn't hurt: you are still able to run your code. Moreover, deprecating warnings are ignored by default (except in the __main__ module). I don't see what is the *practical* issue of deprecating getfullargspec().

getfullargspec() has to die sometimes, it's good to warn users, no :-)

Or is the *long term plan* to keep getfullargspec() forever?

--

I basically have no opinion on these questions. I'm more curious about the long term plan ;-)
msg342362 - (view) Author: mike bayer (zzzeek) * Date: 2019-05-13 17:07
if a function continues to work correctly throughout the span of a Python X version, why does it need to be removed?   I have a foggy memory but I don't seem to recall functions that were technically redundant being aggressively deprecated in the 2.x series.   This warning is causing a lot of downstream pain right now and this is after we already all had to deal with getargspec() going away.
msg342426 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2019-05-14 04:14
RE: "if a function continues to work correctly throughout the span of a Python X version, why does it need to be removed?"

In this case the argument is whether inspect.getfullargspec() works correctly in the face of positional-only parameters. There's also the usual maintenance burden like any other open source project has.

I know for me the thing is that getfullargspec() keeps getting patched up to support any changes we make to function signatures that aren't the smoothest or more accurate way to represent new semantics and so it hasn't been maintenance-free. And in the face of inspect.Signature which feels like a cleaner solution that is easier to improve upon as function signatures change it makes folks not want to keep putting work into the older functions.

I personally think we should at least document the deprecation of the functions to strongly encourage people to not use it in new code. I don't find the "Note that signature() and Signature Object provide the recommended API for callable introspection" message "loud" enough to get the message across.
msg342666 - (view) Author: mike bayer (zzzeek) * Date: 2019-05-16 19:10
> A deprecating warning doesn't hurt: you are still able to run your code.

this is very much untrue in modern testing environments, as it is common that test suites fail on deprecation warnings, especially in libraries, to ensure downstream compatibility.  My observation is that as Python 3 has been generally a lot more aggressive about new language features than things used to be in the py2k days, a lot more of these deprecations have been coming through.  

I've looked at pep 570 and this new syntax does not affect the four libraries for which I have to rewrite getfullargspec, so while I'm looking a bit at things like the "Signature" backport for py2.7 (funcsigs), the pure-Python complexity of Signature is very hard to take on as well as a py2k-only dependency so I'm still leaning towards producing the most minimal getfullargspec() I can use that continues to work for "traditional" Python signatures.

i understand that Signature is "future-proof", but it is heavy on the pure Python and object creation and does not provide for any simple py2k/py3k cross-compatibility solution.   I'd very much prefer that Signature at least be written in C for a few major versions, as well as the community has had time to move further python-3-only,  before getfullargspec is so aggressively deprecated.
msg342667 - (view) Author: Miro Hrončok (hroncok) * Date: 2019-05-16 19:21
As a downstream maintainers of Python in Fedora, this is a great PITA for us. A lot of projects unfortunately treat DeprecationWarnings as errors and we run upstream test suites. It appears that this particular DeprecationWarning is now failing the test suites of the majority of important Python libraries. The warning is there from pluggy (a pytest dependency, already fixed), hypothesis, etc... We mitigate this from the testing framework dependencies, only to realize the tested library itself uses it as well. Current list of problematic packages includes pytest, tornado, sqlalchemy, numpy, dateutil and others.

I'm not saying that should be the only reason not to deprecate stuff (nothing could be deprecated if we stretch that argument too far), but clearly "deprecating warning doesn't hurt" is not an argument either.
msg342669 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-05-16 20:07
For now, I am going to proceed to merge PR13245 and un-deprecate getfullargspec().

Being said that I would want to remark that if test suites are broken when deprecation warnings are emitted that is (1) their own choice, (2) precisely to detect when some deprecation is happening. If we apply some version of this reasoning to everything the standard library will not be able to deprecate anything (not even raise deprecation warnings!). We are talking again and again that we have a lot of old things in the standard library but it seems that removing them is also a problem.

I want to make clear that I understand the arguments here and this is why I am going to merge PR13245 and I also apologize for any pain this may cause, but I also would ask people for empathy towards the standard library, Python and core developers.

Regarding PEP570, getfullargspec() will report positional-only arguments as regular arguments.
msg342670 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-05-16 20:08
New changeset aee19f54f6fe45f6b3c906987941e5a8af4468e9 by Pablo Galindo in branch 'master':
bpo-36751: Undeprecate getfullargspec (GH-13245)
https://github.com/python/cpython/commit/aee19f54f6fe45f6b3c906987941e5a8af4468e9
msg342679 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-16 23:32
Thanks Pablo.

Noting for the record: positional-only arguments aren't new *semantically", since extension modules have always allowed them, and you've long been able to emulate them in Python code by accepting "*args".

Prior to the introduction of argument clinic and __text_signature__, the inspect module wouldn't report *anything* particularly useful for affected signatures, so folks have long built argument introspection based systems around the assumption that they will only be passed functions that they can successfully introspect.

The introduction of a Python level syntax for positional-only arguments doesn't change that logic, as all a consuming application has to do to be compatible with them is to adopt the principle "if an argument can be supplied positionally, it will be supplied positionally", and then only use keywords for keyword-only arguments.

Alternatively, consuming frameworks are also free to make "don't use positional-only arguments" a constraint on the callable inputs they accept.

That said, we *may* want to add a "num_position_only" attribute to FullArgSpec that isn't part of the tuple unpacking, for similar reasons to why we undeprecated getfullargspec in the first place: the easiest way to migrate from getfullargspec to Signature is to write a wrapper API, and if the existence of a wrapper API is inevitable (as I now believe it is, after our experience with the first attempted deprecation), we may as well maintain a properly tested one in the standard library, rather than seeing multiple variants of the same code spring up elsewhere.
msg342683 - (view) Author: mike bayer (zzzeek) * Date: 2019-05-17 01:55
> We are talking again and again that we have a lot of old things in the standard library but it seems that removing them is also a problem.

I agree that the reason we have these deprecation warnings is so that we do get notified and we do fix them.  I think Signature is tough here because it is so different from how getfullargspec() worked which makes it difficult to port towards, and its very long and complicated invocation steps, with many loops, function and method calls, object creation overhead, none of which existed in Python 3.3's 20 line getfullargspec() implementation, is making me very hesitant to switch to it and I'm continuing to consider just writing my own thing that keeps the overhead as low as possible; but I will run timing tests on all versions of things before I do anything like that.
msg342685 - (view) Author: mike bayer (zzzeek) * Date: 2019-05-17 02:31
Just did some benchmarks, and while Signature has apparently had a big speedup in Python 3.6, it is still much less performant than either the Python 2.7 or Python 3.3 implementations, anywhere from 6-18 times slower approximately depending on the function.   For those of us that need a getargspec that is only used on selected, internal functions where we don't need the newer language features, it's likely worth it to vendor the Python 3.3 getfullargspec() implementation which is very simple:

https://gist.github.com/zzzeek/0eb0636fa3917f36ffd887d9f765c208
msg342698 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-17 09:15
Can this issue be closed now? This issue was specific to the impact of the PEP 570 on the inspect module. It's now fixed, right?

If someone wants to continue the discussion about a specific aspect of the inspect module, I suggest to start a thread on python-dev, or open a more specific issue, for example to discuss inspect performance.
msg343187 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-22 12:21
I split https://bugs.python.org/issue37010 out as a separate performance issue in case anyone is inclined to explore the idea of reversing the relationship between inspect.signature and inspect.getfullargspec, such that the latter becomes a fast building block for the former operation, rather than a relatively inefficient compatibility wrapper the way it is now.
History
Date User Action Args
2019-05-22 12:21:34ncoghlansetmessages: + msg343187
2019-05-17 11:23:42pablogsalsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2019-05-17 09:15:45vstinnersetmessages: + msg342698
2019-05-17 02:31:15zzzeeksetmessages: + msg342685
2019-05-17 01:55:53zzzeeksetmessages: + msg342683
2019-05-16 23:32:55ncoghlansetmessages: + msg342679
2019-05-16 20:13:41pablogsalsetpriority: release blocker -> normal
2019-05-16 20:08:32pablogsalsetmessages: + msg342670
2019-05-16 20:07:33pablogsalsetmessages: + msg342669
2019-05-16 19:21:54hroncoksetmessages: + msg342667
2019-05-16 19:10:58zzzeeksetmessages: + msg342666
2019-05-14 04:14:58brett.cannonsetmessages: + msg342426
2019-05-13 17:07:44zzzeeksetmessages: + msg342362
2019-05-13 17:05:41zzzeeksetnosy: + zzzeek
2019-05-13 09:28:26vstinnersetmessages: + msg342296
2019-05-13 09:22:39hroncoksetnosy: + hroncok
messages: + msg342295
2019-05-11 15:23:39pablogsalsetmessages: + msg342197
2019-05-11 15:19:25pablogsalsetpull_requests: + pull_request13157
2019-05-11 04:35:07ncoghlansetmessages: + msg342158
2019-05-11 04:32:18ncoghlansetmessages: + msg342157
2019-05-11 04:24:52ncoghlansetpriority: normal -> release blocker
2019-05-11 04:24:10ncoghlansetmessages: + msg342156
2019-04-30 01:02:54pablogsalsetmessages: - msg341129
2019-04-30 01:02:48pablogsalsetmessages: + msg341130
2019-04-30 01:02:30pablogsalsetpriority: release blocker -> normal

messages: + msg341129
2019-04-30 01:01:16pablogsalsetmessages: + msg341128
2019-04-29 22:35:17pablogsalsetmessages: + msg341124
2019-04-29 22:31:52pablogsalsetkeywords: + patch
stage: patch review
pull_requests: + pull_request12939
2019-04-29 19:43:13lukasz.langasetmessages: + msg341115
2019-04-29 17:49:37brett.cannonsetmessages: + msg341104
2019-04-29 17:45:33xtreaksetnosy: + ncoghlan
2019-04-29 17:32:39steve.dowersetmessages: + msg341102
2019-04-29 17:10:08vstinnersetmessages: + msg341101
2019-04-29 17:02:29brett.cannonsetnosy: + brett.cannon
messages: + msg341098
2019-04-29 14:02:09steve.dowersetpriority: normal -> release blocker
nosy: + lukasz.langa, steve.dower
messages: + msg341083

2019-04-29 13:43:42steve.dowersetnosy: - steve.dower
2019-04-29 13:43:23steve.dowersetpriority: release blocker -> normal
nosy: - lukasz.langa
messages: + msg341082

2019-04-29 13:40:38steve.dowersetpriority: normal -> release blocker

nosy: + lukasz.langa, steve.dower
messages: + msg341081

keywords: + 3.7regression
2019-04-29 13:02:35vstinnersetmessages: + msg341075
2019-04-29 12:13:17pablogsalcreate