Skip to content

gh-108901: Add Signature.from_code method #108902

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions Doc/library/inspect.rst
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,20 @@ function.
.. versionadded:: 3.10
``globalns`` and ``localns`` parameters.

.. classmethod:: Signature.from_code(co)

Return a :class:`Signature` (or its subclass) object
for a given :class:`code object <types.CodeType>` *co*.

.. note::

Default values and annotations
will not be included in the signature,
since code objects have no knowledge of such things.
It is recommended to use :meth:`Signature.from_callable`
when a function object is available, it does not have these limitations.

.. versionadded:: 3.13

.. class:: Parameter(name, kind, *, default=Parameter.empty, annotation=Parameter.empty)

Expand Down
41 changes: 36 additions & 5 deletions Lib/inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -2416,20 +2416,40 @@ def _signature_from_function(cls, func, skip_bound_arg=True,
s = getattr(func, "__text_signature__", None)
if s:
return _signature_fromstr(cls, func, s, skip_bound_arg)

return _signature_from_code(cls,
func.__code__,
globals=globals,
locals=locals,
eval_str=eval_str,
is_duck_function=is_duck_function,
func=func)


def _signature_from_code(cls,
func_code,
globals=None,
locals=None,
eval_str=False,
is_duck_function=False,
func=None):
"""Private helper function to get signature for code objects."""
Parameter = cls._parameter_cls

# Parameter information.
func_code = func.__code__
pos_count = func_code.co_argcount
arg_names = func_code.co_varnames
posonly_count = func_code.co_posonlyargcount
positional = arg_names[:pos_count]
keyword_only_count = func_code.co_kwonlyargcount
keyword_only = arg_names[pos_count:pos_count + keyword_only_count]
annotations = get_annotations(func, globals=globals, locals=locals, eval_str=eval_str)
defaults = func.__defaults__
kwdefaults = func.__kwdefaults__
if func is not None:
annotations = get_annotations(func, globals=globals, locals=locals, eval_str=eval_str)
defaults = func.__defaults__
kwdefaults = func.__kwdefaults__
else:
annotations = {}
defaults = None
kwdefaults = None

if defaults:
pos_default_count = len(defaults)
Expand Down Expand Up @@ -3107,6 +3127,17 @@ def from_callable(cls, obj, *,
follow_wrapper_chains=follow_wrapped,
globals=globals, locals=locals, eval_str=eval_str)

@classmethod
def from_code(cls, co):
"""Constructs Signature for the given code object.

Signatures created from code objects cannot know about annotations
or default values.
"""
if not iscode(co):
raise TypeError(f'code object was expected, got {type(co).__name__!r}')
return _signature_from_code(cls, co)

@property
def parameters(self):
return self._parameters
Expand Down
37 changes: 37 additions & 0 deletions Lib/test/test_inspect.py
Original file line number Diff line number Diff line change
Expand Up @@ -3674,6 +3674,43 @@ def test_signature_on_noncallable_mocks(self):
with self.assertRaises(TypeError):
inspect.signature(mock)

def test_signature_from_code(self):
def func1(
a, b: int,
/,
c, d: str = 'a',
*args: int,
e, f: bool = True,
**kwargs: str,
) -> float:
...

def func2(a: str, b: int = 0, /): ...
def func3(): ...
def func4(*a, **k): ...
def func5(*, kw=False): ...

known_sigs = {
func1: '(a, b, /, c, d, *args, e, f, **kwargs)',
func2: '(a, b, /)',
func3: '()',
func4: '(*a, **k)',
func5: '(*, kw)',
}
Comment on lines +3693 to +3699
Copy link
Member

@AlexWaygood AlexWaygood Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I worry that the reason this wasn't implemented in the original PEP-362 implementation is because the signatures produced for func1, func2 and func5 here are all incorrect, due to the fact that certain parameters have default values but this isn't reflected in the produced signatures :/

inspect.getargs() is used a fair bit in third-party projects. In the vast majority of these uses, they already have access to the function object, so they should probably just use Signature.from_callable() instead of this new method. But there are a few uses where they don't have access to the function object -- so I agree that something like this new method is probably needed, as a replacement for those uses:

Copy link
Member

@AlexWaygood AlexWaygood Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the signatures produced for func1, func2 and func5 here are all incorrect

Hmmm... well, they are actually valid signatures for the code object, since if you pass the code object to exec(), it doesn't take any notice of whether the original function had a default value or not:

>>> def foo(x=42): print(x)
...
>>> foo()
42
>>> c = foo.__code__
>>> exec(c)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: foo() missing 1 required positional argument: 'x'

Why anybody would care what the signature of the code object itself is, though, I'm not sure -- ISTM that everybody trying to get the signature from a code object is just using the code object as the next-best-thing for the function the code object belongs to. I don't think many people are passing code objects directly to exec() It's not even really possible to pass a parameter to a code object you're calling via exec() AFAIK.

Copy link
Member

@AlexWaygood AlexWaygood Sep 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we just need to flag more forcefully in the docs the limitations of this new constructor method. I can offer some suggestions once you've addressed the review comments I've already posted 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a .. note:: notation to highlight it even better.

Copy link
Member

@AlexWaygood AlexWaygood Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about this, the more concerned I am :/

The existing getargs() function that we have in inspect gives accurate information. It doesn't claim to be able to tell you everything about the signature: it only claims to be able to tell you some information about the signature:

  • It gives you a list of argument names
  • It tells you the name of the *vararg argument, if there is one
  • It tells you the name of the **kwarg argument, if there is one

This new function, however, claims to be able to give you a complete and accurate signature from a code object (in that it constructs a Signature object from a code object). That's impossible, since code objects don't have information about which arguments have default values, and so the function gives you inaccurate information.

I'm not sure having a prominent note in the docs is sufficient to fix this issue: adding a new constructor that we know gives inaccurate results seems very troubling to me. Perhaps a better solution would be to document getargs and promote it to public API?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True. This new API would provide helpful information that is not currently offered by getargs.

However. Does it make sense to think of code objects even having a "signature"? Functions have signatures, and you can construct functions from code objects. But as your snippet above demonstrates, you can't directly call code objects (except via exec(), but nobody does that really) -- you have to create a new function from the code object in order to get something that can be called.

Which "signature" are we trying to get here? The signature of the function from which the code object originally came (if it came from a code object), or the signature a new function object would have if we created one from the code object? Getting a "signature from a code object" feels like quite a confusing concept to me.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option: we could document that if people want to get "signature information" from a code object co, they just need to do this:

inspect.signature(types.FunctionType(co, {}))

That way, they are forced to be explicit that code objects don't really have a "signature": you need to construct a new function from the code object in order to get a code object that actually has a signature

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>>> import types, inspect
>>> def a(x: int, /, *, y: str = '') -> None: ...
... 
>>> inspect.signature(types.FunctionType(a.__code__, {}))
<Signature (x, /, *, y)>

It is up to you to decide whether or not we should do it. I consider this PR done from my side.

Copy link
Member

@AlexWaygood AlexWaygood Sep 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, I'm leaning towards not adding this alternative constructor, and just documenting that people should call inspect.signature(types.FunctionType(co, {})) if they need to get signature information from a code object co, and the function object is not available for whatever reason.

One of the third-party projects that uses getargs is Quora: https://github.com/quora/qcore/blob/e12a4929c8e75ec2dd878c06270cedcac72a643e/qcore/inspection.py#L130-L148. I'd be curious to know if @JelleZijlstra has any thoughts on whether getargs should be documented or deprecated and, if the latter, whether we should add this Signature.from_code() method or just add documentation along the lines of my sugestion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any code using that qcore function should really be migrated to use inspect.signature directly. As the docstring notes, the function doesn't deal correctly with kwonly arguments.

I think we don't need Signature.from_code. My experience is that it is rare that you have only a code object without a function it came from, and code that manipulates code objects directly should generally be migrated to a more modern inspect API.


for test_func, expected_sig in known_sigs.items():
with self.subTest(test_func=test_func, expected_sig=expected_sig):
self.assertEqual(
str(inspect.Signature.from_code(test_func.__code__)),
expected_sig,
)

with self.assertRaisesRegex(
TypeError,
"code object was expected, got 'int'",
):
inspect.Signature.from_code(1)

def test_signature_equality(self):
def foo(a, *, b:int) -> float: pass
self.assertFalse(inspect.signature(foo) == 42)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add :meth:`inspect.Signature.from_code` to be able
to construct :class:`inspect.Signature` objects from :class:`types.CodeType`.