I can't see a reason why Signature.from_function() should explicitly check the type of the object being passed in. As long as the object has all required attributes, it should be accepted.

This is specifically an issue with Cython compiled functions, which are not Python functions but look the same.

This patch removes the type check from Signature.from_function() and cleans up the type tests in signature() to use whatever the inspect module defines as isfunction() and isbuiltin(), so that it becomes properly monkey-patchable.

It also adds a test that makes sure the signature relies only on the four relevant special attributes, not on the type of the object being inspected.

Patch looks good, but I’m worried about the change from TypeError to AttributeError in a stable version.

Could you also make clear that all function-like objects are accepted in the doc?

The method doesn't seem to be documented, and I'm not sure if the docstring really benefits from this lengthy addition. Anyway, here's a patch that includes the docstring update.

The exception could be kept the same if we catch an AttributeError and explicitly raise a TypeError instead.

My first concern is whether this is a behavior issue that can be fixed in the next 3.3 release or an enhancement request that should wait for 3.4.

If Signature.from_function were documented in the manual with the current limitation, this issue would definitely be an enhancement request. But it is not even mentioned in the manual. I presume this is intentional as it is pretty clearly not meant to be called directly but only from signature(). Perhaps its should have been given a leading underscore to mark it as private.

The docstring does say 'python function'. However, its type check is redundant with the guard in signature before its call. So in normal usage, it can never fail. Moreover, the limitation is completely unnecessary even for pure Python code, as shown the the ease of creating a funclike class for the test. It is contrary to Python policy and stdlib practice. So in a sense, it is a bug.

My conclusion: the proposed change is an enhancement bugfix (or bugfix enhancement?) for an undocumented private function. So I think it ok for 3.3, but would not blame anyone who disagreed.

---
I agree with Éric that the exception should not be changed, not just because it would be a change, but because is would be a wrong change. Signature.from_function(42) *is* a type error. The fact that the type error is detected by attribute rather than by class is not relevant to external callers. So please wrap the
# Parameter information.
section of the code with try: except AttributeError, as a substitute for the current too-narrow type check. Leave the message as it, except possibly add the object that fails:
"{} is not a function".format(func)

---
The change to signature has no effect on .from_function but makes it consistent with the rest of the module.

Fine with me. Updated patch attached.

Any more comments? Any objections to applying the last patch? Anyone ready to do it?

I certainly think the patch is ok on the principle. I'll let someone else pronounce on the details :-)

I am puzzled by this part of the patch:

• with self.assertRaisesRegex(TypeError, 'is not a callable object'):
  + with self.assertRaisesRegex(TypeError, '42.*is not a callable object'):

With 3.3, I get "TypeError: 42 is not a callable object", which I consider correct, and which should not be changed by the two changes to signature(). So both the old test, missing '42' (how does it pass?) and the new version, with extraneous '.*' look wrong.

I am also puzzled by the 'from None' part in
+ raise TypeError("'{!r}' is not a Python function".format(func)) from None

While I remember that being in the pydev discussion and while "raise XyzError from None" executes, it does not seems to be documented for 3.3 in 7.8. The raise statement. (Should this be another issue?) In fact, that section says " if given, the second expression must be another exception class or instance", which excludes None.

1. that's a regex test, so it just looks for the text. I only extended it to include the object it's supposed to test for. It's not actually related to my changes, but I considered it the right thing to do. I used "42.*is not" in order to allow for alternative spellings like "'42' is not...".

2. "raise ... from None" is the official way to explicitly drop the exception context from a newly raised exception. Otherwise, you'd get two exceptions in this case: a TypeError (as main exception) and an AttributeError (as its context), which I do not consider helpful here as it bloats the output for an otherwise simple error. For 99% of the use cases, it won't matter which attribute was missing.

On Mon, Feb 11, 2013 at 11:46 AM, Terry J. Reedy <report@bugs.python.org> wrote:

I am also puzzled by the 'from None' part in

• raise TypeError("'{!r}' is not a Python function".format(func)) from None

While I remember that being in the pydev discussion and while "raise XyzError from None" executes, it does not seems to be documented for 3.3 in 7.8. The raise statement. (Should this be another issue?) In fact, that section says " if given, the second expression must be another exception class or instance", which excludes None.

That's a separate docs bug - it seems we missed the necessary language
reference updates when implementing PEP-309. The relevant docs are
here: http://docs.python.org/3/library/exceptions.html#built-in-exceptions

I opened separate issue bpo-17188:
Document 'from None' in raise statement doc.

Updated patch to also fix a little typo in the docstring (lower case "python").

Stefan, is this one still relevant?

If it is, I can review it (I have a couple of things to improve) and push in 3.4, as it doesn't really change any API.

Yes, the situation didn't change.

Stefan,

I'm attaching a reviewed patch--was a bit easier to rebase it on the new code and fix the remaining issues.

Please review.

BTW, do cython functions have __name__ attribute?

LGTM, thanks!

And works nicely with Cython:

"""
import inspect

def test_isfunction():
    """
    >>> test_isfunction()
    True
    """
    return inspect.isfunction(test_isfunction)

def test_signature():
    """
    >>> test_signature()   # doctest: +ELLIPSIS
    <inspect.Signature object ...>
    """
    return inspect.signature(test_signature)
"""

BTW, do cython functions have __name__ attribute?

Yes, they have everything that can possibly be emulated (although not
everything currently contains useful information). If your question is
whether that should be tested for, too, then I'd say yes.

Attached is a second patch (sig_func_ducktype_02.patch), with a bit improved tests (and a small bug fixed).

Larry, do you think it's OK if I merge this one in 3.4? Technically, it's a feature that we start to support Cython functions in inspect.signature, but from the standpoint of the API everything is the same.

pretty please? :)

Tell you what. If this *specifically* is a fix for Cython, please add a some text saying that to the comment for the duck-type test function.

If you do that, and assuming that test_inspect still passes, then you may check it in.

Third version of the patch is attached (sig_func_ducktype_03.patch).

Changes:

• Toughen the tests for duck-typed function -- do not accept classes
• Added comments to clarify the need of duck-typing
• NEWS item
• what's new item

Stefan, please take a look. I want to make sure, that the fact that we won't accept classes that look like a function, is OK with you.

Tried it, works for me.

Explicitly rejecting classes is a good idea, IMHO, as is requiring that any function-like object must be callable, obviously.

Explicitly rejecting classes is a good idea, IMHO, as is requiring that any function-like object must be callable, obviously.

Yeah, I think it's good to restrict this duck-typing as much as possible. Committing the patch.

New changeset 32a660a52aae by Yury Selivanov in branch 'default':
inspect.signature: Support duck-types of Python functions (Cython, for instance) bpo-17159
http://hg.python.org/cpython/rev/32a660a52aae

OK, closing this one.
Stefan, Larry, thank you for your reviews and time.

Also, PEP-8 forbids using annotations in the CPython library, which
includes all of CPython's builtins. So using annotations in any way
for this was off the table.

Interesting, wasn't aware of that. Then let's wait what will happen when
users start manually adding signature information to the docstrings of
their C code extensions and come asking for annotation support in order to
allow for introspection of the expected parameter types.

You imply that __text_signature__ is improper and unsafe.

What I'm saying is that the existing function introspection API would have
provided a much better way to do these things, and that it's good to
finally have the meta data available in the source code so that that API
can be made available at some point. Sorry if my wording sounded offensive.

If in the future we need to change how we represent signatures, we can
remove it. It's an undocumented internal implementation detail and we
have free license to do so.

I don't think that's possible any more once it's out in the wild. The
feature is just way too visible.

> inspect.isbuiltin() returns False
Are you absolutely sure about this?

Yes. The "inheritance" of Cython's function type from PyCFunction is a pure
implementation detail of the object struct layout that is not otherwise
visible in any way. Specifically, there is no base type. The only reason
for the identical (start of the) struct layout is to allow reusing some of
the normal PyCFunction C-API functions on it instead of having to copy them
into Cython.

Cython's functions are neither instances of PyFunction nor of PyCFunction.
They implement the interface of PyFunction, though.

http://hg.python.org/cpython/rev/48c3c42e3e5c

This change is redundant since BuiltinFunctionType (which isbuiltin() tests
for) is already in _NonUserDefinedCallables, which is tested for right
afterwards.

This change is redundant since BuiltinFunctionType (which isbuiltin() tests
for) is already in _NonUserDefinedCallables, which is tested for right
afterwards.

The code is more understandable, though.
Anyways, I'm glad that the issue is now resolved.

Also, please let's stop posting anything unrelated in this issue.

What I'm saying is that the existing function introspection API
would have provided a much better way to do these things,
and that it's good to finally have the meta data available in the
source code so that that API can be made available at some point.

What "existing function introspection API"? I wasn't aware there was an existing mechanism to provide signature metadata for builtin functions.

What "existing function introspection API"? I wasn't aware there was an
existing mechanism to provide signature metadata for builtin functions.

Not for builtin functions, but it's unclear to me why the API of builtin
functions should be different from that of Python functions (except, as I
said, for the existence of byte code).

I agree with Yury, however, that this discussion is unrelated to this ticket.

Not for builtin functions, but it's unclear to me why the API of
builtin functions should be different from that of Python functions
(except, as I said, for the existence of byte code).

I really don't follow you. You seem to be saying that __text_signature__ is a bad idea, and keep talking about existing
APIs that provide for the same functionality, but you decline to name
specifics.

Be specific. Let's say we remove __text_signature__. How do we
now write a C extension in a way that we can have introspection
information for its callables?

If __text_signature__ is redundant with existing APIs, then we should remove it now before 3.4 ships.

Python 3.4.0b3+ (default:19d81cc213d7, Feb  1 2014, 10:38:23)
[GCC 4.8.1] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def test(a,b,c=None): pass
>>> set(dir(test)) - set(dir(len))
{'__get__', '__code__', '__globals__', '__dict__', '__defaults__', '__kwdefaults__', '__annotations__', '__closure__'}
>>> test.__kwdefaults__
>>> test.__defaults__
(None,)
>>> test.__annotations__
{}
>>> test.__code__
<code object test at ..., file "<stdin>", line 1>
>>> dir(test.__code__)
['__class__', '__delattr__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__gt__', '__hash__', '__init__', '__le__', '__lt__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', 'co_argcount', 'co_cellvars', 'co_code', 'co_consts', 'co_filename', 'co_firstlineno', 'co_flags', 'co_freevars', 'co_kwonlyargcount', 'co_lnotab', 'co_name', 'co_names', 'co_nlocals', 'co_stacksize', 'co_varnames']
>>> test.__code__.co_varnames
()
>>> test.__code__.co_varnames
()
>>> test.__code__.co_varnames
('a', 'b', 'c')
>>> test.__code__.co_kwonlyargcount
0
>>> test.__code__.co_name
'test'

But again, this is not covered by the subject of this ticket. I also don't think it's a good idea to delay Py3.4 until this discrepancy is fixed.

"""
>>> test.__code__.co_varnames
()
>>> test.__code__.co_varnames
()
>>> test.__code__.co_varnames
('a', 'b', 'c')
"""

copy&pasto, please ignore the first two... :o)

Yyou have just answered the question "How do you determine signature information for functions written in Python?". A shorter way to express this answer: functions written in Python are implemented as a "function object" (in C, PyFunctionObject), which internally has a reference to a "code object" (PyCodeObject). These two objects collectively contain all the information you'd need to determine the function's signature in Python.

However, builtin functions don't use either of these objects. Builtin functions are implemented with a "builtin code object" (PyCFunctionObject) which doesn't have any of the metadata you cited. So that doesn't answer my question. Nor is it practical to implement a builtin function using a "function object" and a "code object".

So I'll ask you again:

Be specific. Let's say we remove __text_signature__. How do we
now write a C extension in a way that we can have introspection
information for its callables?

[...] a "builtin code object" (PyCFunctionObject) [...] doesn't have any of the metadata you cited.

That exactly is the bug.

I think we should take the further discussion offline, or move it to a new ticket.

Stefan is suggesting that rather than emulating the Python signature line
(which is a concise human readable notation that with the PEP-457 tweaks
will handle even the most obscure custom argument parsing, whether that
parsing is implemented in C or in Python), it would make more sense to
instead emulate the confusing jumble of CPython implementation details
exposed by Python level function and code objects. That ducktyping has been
the traditional way to support signature introspection for anyone that
didn't have the ability to change how the inspect module works.

However, I'm not sure *how* generating and storing an assortment of hard to
interpret lists and strings and mappings would qualify as being simpler
than generating and storing a single string that is only converted into
real signature data if requested. Once __text_signature__ becomes a public
API in 3.5 (as part of PEP-457), inspect shouldn't need to special case any
type ever again: it will be up to the type to set __text_signature__
properly instead.

I understand Stefan to (reasonably) want 1 api instead of 2. He imagined that the only way to do that would be for everything to at least partially imitate the old methods. Larry and Nick are suggesting instead that everything should adopt the implementation-independent new method. Sounds right to me. So further discussion should be about PEP-457, elsewhere.

I understand Stefan to (reasonably) want 1 api instead of 2.

Absolutely. However, I guess the underlying reasoning here is that there
are other callables, too, so it's not worth making just two kinds of them
look the same, even if both are functions and even if both are part of
CPython itself.

As soon as there is easy-to-use support for annotations in C implemented
functions (for use outside of CPython and Cython), I'll agree that the need
for changing anything in CPython's builtin functions type isn't really
worth bothering about any more in the future, given that the Signature
object is the designated unifying API.

>> inspect.isbuiltin() returns False
> Are you absolutely sure about this?
Yes.

Oh, well...

isbuiltin(cyfunction) *does* return False. However, ismethoddescriptor(cyfunction) returns True, because Cython's functions bind as methods and thus have a "__get__()" (but no __set__()). Therefore, _signature_is_builtin(cyfunction) returns True, and the Signature.from_builtin(cyfunction) code path is taken and fails.

So, once again:

If the intention is to test that a callable has a "__text_signature__", then the code should test for the existance of "__text_signature__". It should *not* try to test for types that may or may not have such an attribute, and then just fail if it surprisingly does not find it.

BTW, ismethoddescriptor() is an exceedingly broad test, IMHO. I wonder if it should even be relied upon for anything in inspect.py.

Since this is a problem in Cython, not in CPython, maybe you can fix it in Cython?

Patch sig_cython_03.patch is still applicable (it probably won't apply to the current repo, but I can fix that). It should solve all these problems once and for all.

If Larry agrees, I can still merge it in.

So, once again:

If the intention is to test that a callable has a "__text_signature__",
then the code should test for the existance of "__text_signature__".
It should *not* try to test for types that may or may not have such
an attribute, and then just fail if it surprisingly does not find it.

Unfortunately this is something I do *not* want to change.

Since this is a problem in Cython, not in CPython, maybe you can fix it in Cython?

I'm actually considering that. Now that Signature.from_function() allows
function-like types, it appears like it's the easiest solution to add a
"__signature__" property to cyfunctions that does the necessary "from
inspect import Signature, return Signature.from_function(self)" dance.

I'm actually considering that. Now that Signature.from_function() allows
function-like types, it appears like it's the easiest solution to add a
"__signature__" property to cyfunctions that does the necessary "from
inspect import Signature, return Signature.from_function(self)" dance.

Oh, sound like a big hack.

Stefan, Larry,

Please take a look at the new patch 'sig_cython_latest'

The change is really minimal, I think we can still push this in 3.4.

Oh, sound like a big hack.

Well, it's certainly a bunch of overhead (even assuming that "inspect" will
most likely be imported already - just looked it up in import.c, there's
lots of useless generic code there), with a lot of potential for something
going wrong, but it should still be somewhat acceptable. Certainly not time
critical. Having to maintain our own function type isn't exactly the most
simple way of going about it in the first place.

I just tried it, it adds some 20 lines of C code but works ok.

Please take a look at the new patch 'sig_cython_latest'
The change is really minimal, I think we can still push this in 3.4.

I'm certainly ok with it, given that I had already asked for that a while ago.

New changeset d6aa3fa646e2 by Yury Selivanov in branch 'default':
inspect.signature: Check for function-like objects before builtins. Issue bpo-17159
http://hg.python.org/cpython/rev/d6aa3fa646e2

Stefan,

I committed one more fix in signature -- now cython
functions should be finally supported.

I'm not sure if Larry accepts this in 3.4.0 though,
but I'll create an issue for that.

I'm closing this issue. Please do not reopen it, and create a new one if necessary.

Stefan: does the checkin in d6aa3fa646e2 fix your problem? I'm willing to cherry-pick for this but I'd prefer to only have to do it once.

I tested it and it works, so I could take the simple route now and say "yes, it fixes the problem", but it's actually no longer required because I already added a "__signature__" property to Cython's functions. However, as Yury noted, that's a hack because inspect.py can do the same thing way more efficiently with his latest change, so it allows me to reconsider and potentially get rid of it again.

Long story short, it works and does the right thing, so I'm happy to see this change go into Py3.4.

New changeset fa5127cdfe9d by Yury Selivanov in branch '3.4':
inspect.signature: Check for function-like objects before builtins. Issue bpo-17159
http://hg.python.org/cpython/rev/fa5127cdfe9d