gh-108901: Add `bound_arg` to `Signature.from_callable()` and `signature()` #116559

sobolevn · 2024-03-10T11:52:01Z

Issue: Add modern alternatives to inspect module, deprecate old incorrect APIs #108901

📚 Documentation preview 📚: https://cpython-previews--116559.org.readthedocs.build/

…and `signature()`

Doc/library/inspect.rst

Doc/whatsnew/3.13.rst

Lib/test/test_inspect/test_inspect.py

Co-authored-by: Victor Stinner <[email protected]>

Lib/test/test_inspect/test_inspect.py

vstinner · 2024-03-11T09:53:22Z

By the way, skip_bound_arg parameter name is a little bit long. Maybe you can rename it to bound_arg.

@pitrou: Do you have an opinion on the parameter name? :-)

sobolevn · 2024-03-11T09:55:50Z

I would prefer to have skip_bound_arg as a name, because it has a history (from existing code in inspect.py):

cpython/Lib/inspect.py

Lines 1404 to 1413 in 4e5df20

 # Re: `skip_bound_arg=False` 

 # 

 # There is a notable difference in behaviour between getfullargspec 

 # and Signature: the former always returns 'self' parameter for bound 

 # methods, whereas the Signature always shows the actual calling 

 # signature of the passed object. 

 # 

 # To simulate this behaviour, we "unbind" bound methods, to trick 

 # inspect.signature to always return their first parameter ("self", 

 # usually)

vstinner · 2024-03-11T09:59:49Z

skip_bound_arg=False is a double negation, and I'm always confused by double negation :-( bound_arg=True is more straightfoward to me.

Previously, the argument was private, no?

sobolevn · 2024-03-11T10:11:07Z

Yes, it was private. It only existed on protected _signature_from_callable helper.

Doc/library/inspect.rst

sobolevn · 2024-04-24T13:51:22Z

Done, now it is named bound_arg! 👍

encukou · 2024-04-25T14:24:01Z

What's the motivation for this change? I can't find the discussion behind it.

My worry is that it only works on some callables, and it's not very clear which ones those are -- or how to make a custom callable (e.g. one implemented in the C API) work like a method.

sobolevn · 2024-04-25T14:33:41Z

What's the motivation for this change?

I want to deprecate getfullargspec, which works almost like signature(), but with this difference. See

cpython/Lib/inspect.py

Lines 1403 to 1423 in 2c45148

 # Re: `skip_bound_arg=False` 

 # 

 # There is a notable difference in behaviour between getfullargspec 

 # and Signature: the former always returns 'self' parameter for bound 

 # methods, whereas the Signature always shows the actual calling 

 # signature of the passed object. 

 # 

 # To simulate this behaviour, we "unbind" bound methods, to trick 

 # inspect.signature to always return their first parameter ("self", 

 # usually) 

 # Re: `follow_wrapper_chains=False` 

 # 

 # getfullargspec() historically ignored __wrapped__ attributes, 

 # so we ensure that remains the case in 3.3  

 sig = _signature_from_callable(func, 

 follow_wrapper_chains=False, 

 skip_bound_arg=False, 

 sigcls=Signature, 

 eval_str=False)

My worry is that it only works on some callables, and it's not very clear which ones those are -- or how to make a custom callable (e.g. one implemented in the C API) work like a method.

It will work exactly like this older function.

encukou · 2024-04-25T14:51:14Z

Is this behaviour useful, or is it a bug that's kept in getfullargspec for backwards compatibility?

ncoghlan · 2024-04-29T15:08:14Z

Doc/library/inspect.rst

+ If *bound_arg* is ``False``, remove ``self`` parameter
+ from the method signature.


It's better to describe the non-default behaviour for boolean options (so folks know what they'll get if they request non-default behaviour):

Suggested change

If *bound_arg* is ``False``, remove ``self`` parameter

from the method signature.

If *bound_arg* is ``True``, report the (already bound) first parameter on bound

instance or class methods (usually ``self`` or ``cls``). This emulates the historical

behaviour of the deprecated :func:`getfullargspec` function.

(Describing both behaviours is also an option, but it seemed excessively verbose in this case)

ncoghlan · 2024-04-29T15:15:52Z

Doc/whatsnew/3.13.rst

+
+* Add *bound_arg* parameter to :func:`inspect.Signature.from_callable`
+ and :func:`inspect.signature`: keep the ``self`` parameter
+ in the method signature if *bound_arg* is True.


Reword here to match the updated main function docs (including mentioning the getfullargspec compatibility)

ncoghlan · 2024-04-29T15:19:56Z

Doc/whatsnew/3.13.rst

+* Add *bound_arg* parameter to :func:`inspect.Signature.from_callable`
+ and :func:`inspect.signature`: keep the ``self`` parameter
+ in the method signature if *bound_arg* is True.
+ :pypi:`inspect313` package has a backport of this feature.


Maybe make the backport mention a seealso note in the module docs rather than only putting it here?

ncoghlan · 2024-04-29T15:20:44Z

Misc/NEWS.d/next/Library/2024-03-10-14-45-00.gh-issue-108901.vMlWej.rst

@@ -0,0 1,3 @@
+Add *bound_arg* keyword-only parameter to
+:func:`inspect.Signature.from_callable` and :func:`inspect.signature`.
+If *bound_arg* is ``True``, keep ``self`` parameter in method a signature.


Mention the getfullargspec compatibility aspect here.

vstinner · 2024-04-29T15:29:42Z

Is this behaviour useful, or is it a bug that's kept in getfullargspec for backwards compatibility?

IMO it's useful in general, not only for backward compatibility. Keep self or not should be configurable.

ncoghlan · 2024-04-29T15:41:04Z

Is this behaviour useful, or is it a bug that's kept in getfullargspec for backwards compatibility?

I think it was primarily a quirk of the way getfullargspec was originally implemented (i.e. it followed bound methods down to the underlying function to get the signature details, and then just returned those details as is without accounting for the first parameter already being bound).

However, the suggested flag isn't hard to maintain, since the complexity is in the branch that corrects the underlying function signature to remove the already bound first parameter (the default behaviour). The "unwrap bound methods" branch is literally just return sig:

cpython/Lib/inspect.py

Line 2517 in 3989894

if skip_bound_arg:

I do think making the flag public is worth including as a step prior to full programmatic deprecation of getfullargspec. With the suggested docs change, I also think the bound_arg parameter name is a reasonable one.

(The one potential argument I see for a parameter name like unbind_methods over bound_arg, is that the flag specifically affects the handling of the bound arg in bound methods. It doesn't have any effect on the handling of the previously bound arguments in functools.partial and functools.partialmethod instances. However, I don't think that concern is significant enough to be worth changing the parameter name again)

encukou · 2024-04-30T09:11:19Z

the suggested flag isn't hard to maintain

My worry is that while the flag isn't hard to maintain as an internal detail of how getfullargspec extracts information from Python functions/methods, signature should work for other callables, like instances of C extension types (e.g. ones with Py_TPFLAGS_METHOD_DESCRIPTOR), or Cython functions. We might not be able to support these things transparently, but there should be a clear way to make custom objects that work with the flag. Or a documented way, at least.

(Unfortunately I'll probably not have time to investigate this worry before beta 1. If the feature is waiting for my review, it won't make it.)

pythongh-108901: Add skip_bound_arg to Signature.from_callable() …

290e1ad

…and `signature()`

sobolevn requested review from gpshead, vstinner, hugovk and larryhastings March 10, 2024 11:52

bedevere-app bot added the awaiting core review label Mar 10, 2024

bedevere-app bot mentioned this pull request Mar 10, 2024

Add modern alternatives to inspect module, deprecate old incorrect APIs #108901

Open

vstinner reviewed Mar 10, 2024

View reviewed changes

Doc/library/inspect.rst Outdated Show resolved Hide resolved

Doc/whatsnew/3.13.rst Outdated Show resolved Hide resolved

Lib/test/test_inspect/test_inspect.py Outdated Show resolved Hide resolved

sobolevn and others added 3 commits March 10, 2024 19:37

Update Doc/whatsnew/3.13.rst

2e36181

Co-authored-by: Victor Stinner <[email protected]>

Address review

a0ed116

Update docs

470625f

vstinner reviewed Mar 10, 2024

View reviewed changes

Lib/test/test_inspect/test_inspect.py Outdated Show resolved Hide resolved

sobolevn added 2 commits March 11, 2024 12:15

Show the signature change in test

8f6424a

Simply example

4f08ab7

vstinner reviewed Mar 11, 2024

View reviewed changes

Lib/test/test_inspect/test_inspect.py Outdated Show resolved Hide resolved

Lib/test/test_inspect/test_inspect.py Outdated Show resolved Hide resolved

Lib/test/test_inspect/test_inspect.py Outdated Show resolved Hide resolved

Improve readability

d76ba71

skirpichev reviewed Apr 2, 2024

View reviewed changes

Doc/library/inspect.rst Outdated Show resolved Hide resolved

sobolevn added 2 commits April 24, 2024 16:36

Merge branch 'main' into issue-108901-add-getfullargspec

e30aeb2

Rename skip_bound_arg to bound_arg

4d5a96e

sobolevn changed the title ~~gh-108901: Add skip_bound_arg to Signature.from_callable() and signature()~~ gh-108901: Add bound_arg to Signature.from_callable() and signature() Apr 24, 2024

Mention inspect313

3b884eb

ncoghlan reviewed Apr 29, 2024

View reviewed changes

hugovk removed their request for review April 30, 2024 07:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-108901: Add `bound_arg` to `Signature.from_callable()` and `signature()` #116559

gh-108901: Add `bound_arg` to `Signature.from_callable()` and `signature()` #116559

sobolevn commented Mar 10, 2024 •

edited by github-actions bot

Loading

vstinner commented Mar 11, 2024

sobolevn commented Mar 11, 2024 •

edited

Loading

vstinner commented Mar 11, 2024

sobolevn commented Mar 11, 2024

sobolevn commented Apr 24, 2024

encukou commented Apr 25, 2024

sobolevn commented Apr 25, 2024

encukou commented Apr 25, 2024

ncoghlan Apr 29, 2024

ncoghlan Apr 29, 2024

ncoghlan Apr 29, 2024

ncoghlan Apr 29, 2024

vstinner commented Apr 29, 2024

ncoghlan commented Apr 29, 2024 •

edited

Loading

encukou commented Apr 30, 2024

		If bound_arg is ``False``, remove ``self`` parameter
		from the method signature.

- If *bound_arg* is ``False``, remove ``self`` parameter
- from the method signature.
+ If *bound_arg* is ``True``, report the (already bound) first parameter on bound
+ instance or class methods (usually ``self`` or ``cls``). This emulates the historical
+ behaviour of the deprecated :func:`getfullargspec` function.

gh-108901: Add bound_arg to Signature.from_callable() and signature() #116559

Are you sure you want to change the base?

gh-108901: Add bound_arg to Signature.from_callable() and signature() #116559

Conversation

sobolevn commented Mar 10, 2024 • edited by github-actions bot Loading

vstinner commented Mar 11, 2024

sobolevn commented Mar 11, 2024 • edited Loading

vstinner commented Mar 11, 2024

sobolevn commented Mar 11, 2024

sobolevn commented Apr 24, 2024

encukou commented Apr 25, 2024

sobolevn commented Apr 25, 2024

encukou commented Apr 25, 2024

ncoghlan Apr 29, 2024

Choose a reason for hiding this comment

ncoghlan Apr 29, 2024

Choose a reason for hiding this comment

ncoghlan Apr 29, 2024

Choose a reason for hiding this comment

ncoghlan Apr 29, 2024

Choose a reason for hiding this comment

vstinner commented Apr 29, 2024

ncoghlan commented Apr 29, 2024 • edited Loading

encukou commented Apr 30, 2024

gh-108901: Add `bound_arg` to `Signature.from_callable()` and `signature()` #116559

gh-108901: Add `bound_arg` to `Signature.from_callable()` and `signature()` #116559

sobolevn commented Mar 10, 2024 •

edited by github-actions bot

Loading

sobolevn commented Mar 11, 2024 •

edited

Loading

ncoghlan commented Apr 29, 2024 •

edited

Loading