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 #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) #17159
http://hg.python.org/cpython/rev/32a660a52aae

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

Thanks a lot!

New changeset 8a91132ed6aa by Yury Selivanov in branch 'default':
inspect.Signauture.from_function: validate duck functions in Signature constructor #17159
http://hg.python.org/cpython/rev/8a91132ed6aa

Hmm, I now notice that I was mistaken about this working:

'''
import inspect

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

It only worked in Cython's test suite because its test runner monkey patches "inspect.isfunction", and I had completely forgotten about it. Sorry for the confusion.

The thing is that Cython's function type isn't really a Python function (obviously), it inherits from PyCFunction, so it should return True for isbuiltin(). A problem on our side prevented that. If I fix it up, then the newly added duck-typing code actually ends up not being used, because signature() tests for isbuiltin() first and runs into Signature.from_builtin(), which is the Argument Clinic code path that expects a textual signature representation. Cython functions don't have that, because they are compatible with Python functions.

This situation could be helped in inspect.signature() by reversing the test order, i.e. by changing this code

    if _signature_is_builtin(obj):
        return Signature.from_builtin(obj)

    if isfunction(obj) or _signature_is_functionlike(obj):
        # If it's a pure Python function, or an object that is duck type
        # of a Python function (Cython functions, for instance), then:
        return Signature.from_function(obj)

into this:

    if isfunction(obj) or _signature_is_functionlike(obj):
        # If it's a pure Python function, or an object that is duck type
        # of a Python function (Cython functions, for instance), then:
        return Signature.from_function(obj)

    if _signature_is_builtin(obj):
        return Signature.from_builtin(obj)

Would this be ok?

I would also argue that the implementation of _signature_is_builtin() is, well, not ideal, because what it should test for according to the comment at the top of the function is the existance of "__text_signature__". Instead, it does several type tests, one of which goes wrong in this case.

> Would this be ok?

Probably. I need to take a closer look.

I'm not sure I like the idea that Cython functions are "chimeras" of some sort, i.e. they have a type of python builtin functions, hence, logically, Signature.from_builtin should work on them (and they have to follow __text_signature__ API), and on the other hand, they try to mimic pure python functions (being a builtin type) with all its guts like '__code__' object etc.  

Perhaps, what we need to do, is to modify 'Signature.from_builtin' to check for pure-python function duck type too, and fallback to 'Signature.from_function' in this case. Larry, Nick, what do you think?

> I would also argue that the implementation of _signature_is_builtin() is, well, not ideal, because what it should test for according to the comment at the top of the function is the existance of "__text_signature__". Instead, it does several type tests, one of which goes wrong in this case.

'from_builtin' needs to have those type checks. Duck typing is good, but some minimal type safety is good too.

> I'm not sure I like the idea that Cython functions are "chimeras" of
> some sort, i.e. they have a type of python builtin functions, hence,
> logically, Signature.from_builtin should work on them (and they have to
> follow __text_signature__ API), and on the other hand, they try to mimic
> pure python functions (being a builtin type) with all its guts like
> '__code__' object etc.

That's one way of looking at it. The way I see it is that CPython's builtin
functions should rather behave exactly like Python functions. The fact that
there is such a thing as a "__text_signature__" and general special casing
of builtins is IMHO a rather annoying but truly long standing bug. The only
necessary difference is that one of them contains byte code and the other
doesn't, everything else should eventually be aligned.

> Perhaps, what we need to do, is to modify 'Signature.from_builtin' to
> check for pure-python function duck type too, and fallback to
> 'Signature.from_function' in this case.

In any case, I think that a complete Python function(-like) interface
should always be preferred to work-arounds like "__text_signature__",
regardless of where it comes from.

> 'from_builtin' needs to have those type checks. Duck typing is good, but
> some minimal type safety is good too.

I don't really see why. The code doesn't seem to be doing that much more
than text processing of the "__text_signature__", plus a tiny bit of
optional(!) attribute checking ("__module__" and "__self__").

The restrictive type checks appear to be the only thing that prevents users
from doing this:

    class myfunc:
        __text_signature__ = '(a,b,c,d)'

    sig = Signature.from_builtin(myfunc())

Granted, the name of that method doesn't really fit well in that case, and
a simpler interface than having to define a class would also not hurt.

> That's one way of looking at it. The way I see it is that CPython's builtin
> functions should rather behave exactly like Python functions. The fact that
> there is such a thing as a "__text_signature__" and general special casing
> of builtins is IMHO a rather annoying but truly long standing bug. The only
> necessary difference is that one of them contains byte code and the other
> doesn't, everything else should eventually be aligned.

I see your point. I think that modifying 'from_builtin' as I suggested in my previous comment is the right thing to do. I'll make a new patch soon.

Attached is a minimal patch that does what I think you meant.

Stefan,
Please try the attached patch (sig_cython_01.patch)

I tried the third patch and it works, but when I write this into a docstring:

    def func(x, *, y=None):
        """sig=(a,b)"""

then it fails to extract the signature again and returns (a,b) instead.

I also tried putting in some math term as (non-signature) documentation:

    def sig(a, b):
        """sig=(a*b)"""

and it raises a ValueError telling me that the signature is invalid.

So, I still think it should prefer a full function interface over the risky
"__text_signature__" quirk whenever it can.

I also think it shouldn't raise an error if the docstring isn't something
that it can parse, but that's not part of this ticket, I guess.

In case a little background would help: while developing support for '__text_signature__' I had to move the test and the from_builtin() call to the top.  It used to be more in the middle-ish.  I don't have notes specifically on why I moved it, but I dimly recall that if I didn't try from_builtin first, there were other approaches that would succeed on the callable, that approach would fail on a builtin, so it'd throw an exception, and from_builtin wouldn't get a chance to try.

Also, I assumed that anything that had a __text_signature__ wasn't going to have any other kind of signature information.  Therefore, if the object had a __text_signature__ attribute, then I already knew that's the best approach and it should definitely be first.

Also also, I remember specifically that the isinstance(type) code would fail builtin classes.  But there's no generic way (that I know of) to tell whether a class is a user class or a builtin class.  So I wanted from_builtin to try handling a class first before the isinstance(type) class.

Are there callables in CPython that have *both* a __text_signature__ *and* have signature information derivable from another source, where you *don't* want to use __text_signature__?


Stefan: yes, you can write any garbage you want after "sig=(" and the C function that detects signatures will still assume you have a text signature.  That reminds me of a good joke.

Patient: Doctor, it hurts whenever I move my arm in this funny way.  *demonstrates*
Doctor: Well, then, don't do that.

I would be very interested if you knew of docstrings in the wild that innocently start with "sig=(" and yet aren't intended to be text signatures compatible with inspect.Signature.

> I assumed that anything that had a __text_signature__ wasn't going
> to have any other kind of signature information.  Therefore, if the
> object had a __text_signature__ attribute, then I already knew that's
> the best approach and it should definitely be first.

That's likely true for CPython itself, at least currently. I don't think
it's generally a safe assumption in the CPython ecosystem, which includes
things like Cython, Numba, Nuitka and others, i.e. a growing number of
tools that can create function-like native callables with a signature that
is worth introspecting. They may or may not provide a function-like
signature, but if they do (and at least Cython and Nuitka provide one),
then that's the best you can get. Reducing it to a string representation,
especially now that we have annotations for Python signatures, sounds like
a major step backwards.

> Are there callables in CPython that have *both* a __text_signature__
> *and* have signature information derivable from another source, where
> you *don't* want to use __text_signature__?

Anything that inherits from PyCFunction will inherit "__text_signature__"
automatically. I should mention that this inheritance is actually not
allowed, according to the type flags in PyCFunction_Type. It's just that
Cython has been doing it that way for years, mainly because there are some
barriers in CPython (don't remember which) that prevent other types from
doing similar things.

> I would be very interested if you knew of docstrings in the wild that
> innocently start with "sig=(" and yet aren't intended to be text
> signatures compatible with inspect.Signature.

I agree that it's an unlikely start for a docstring. That's why it was
chosen, after all.

Safer ways to do it would be extending the type, or adding a flag in some
way, but that's going to a) hurt more than the current situation, and b)
become wasteful at some point when the __text_signature__ actually gets
replaced by a proper function interface for CPython builtins. The fact that
that wasn't doable for Py3.4 any more doesn't mean it shouldn't be done at all.

Ok, I think I figured it out now.

Essentially, Cython's functions type, despite starting with the same struct layout as PyCFunction, must not visibly inherit from PyCFunction. Consequently, inspect.isbuiltin() returns False, as does inspect.isfunction() - sadly.

With Yury's changes that are currently committed, this triggers the fallback that checks for the function-like signature, which in turn makes Signature.from_function() properly analyse the signature, including default arguments, annotations, etc.

So, nothing left to change on CPython side for this ticket. And thanks again for the help. Closing as fixed.

Stefan,

> inspect.isbuiltin() returns False

Are you absolutely sure about this? Please check that everything works with this latest change http://hg.python.org/cpython/rev/48c3c42e3e5c

Larry,

> Also, I assumed that anything that had a __text_signature__ wasn't going to have any other kind of signature information.  Therefore, if the object had a __text_signature__ attribute, then I already knew that's the best approach and it should definitely be first.

I think that __text_signature__ is similar to the __signature__ attribute, i.e. if an object has it, then it should blindly use it, without trying anything else (where __signature__ should have a higher priority than __text_signature__)

> That's likely true for CPython itself, at least currently.
> I don't think it's generally a safe assumption in the CPython
> ecosystem, which includes things like Cython, Numba, Nuitka and others,

While I'm happy to accommodate these other projects if it's easy to do
so, my primary concern is what's best for CPython.


> Reducing it to a string representation, especially now that we
> have annotations for Python signatures, sounds like a major step
> backwards.

What nonsense.  CPython has never had signature information for
builtins before.  And, the format of __text_signature__ allows
expressing a signature that uses every feature available in
inspect.Signature.  This cannot legitimately be called a "major step
backwards".

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.


> Safer ways to do it would be extending the type, or adding a flag
> in some way, but that's going to a) hurt more than the current
> situation, and b) become wasteful at some point when the
> __text_signature__ actually gets replaced by a proper function
> interface for CPython builtins. The fact that that wasn't doable
> for Py3.4 any more doesn't mean it shouldn't be done at all.

You imply that __text_signature__ is improper and unsafe.  We debated
how to communicate static signature information from C to Python at
runtime, and this was easily the best--most expressive, most
compact.  So it is entirely proper.  Hiding it in the docstring was
a cheap hack, I'll admit, but with an appropriately unlikely signature
('sig=') it seems perfectly safe.

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.  So even if we changed our mind in the
future I don't see how it would be "wasteful".

> 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 #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 #17159
http://hg.python.org/cpython/rev/fa5127cdfe9d