Stefan added some docstring text signatures by hand, only to discover that inspect.Signature still didn't recognize them.  Specifically, decimal.Decimal.compare was unrecognized.  This is a method_descriptor object, which is a type that isn't even exposed in types.

Rather than go on a search-and-destroy mission for all these different builtin types, I'm going to change inspect.Signature so as a fallback at the end it says "if it has a __call__ and a valid __text_signature__, just use that".

Okay, learned some things.

1) inspect already has an ismethoddescriptor().  So I'll try to do this properly by 

2) The real problem is that method_descriptor doesn't have __text_signature__.  I only added that to PyCFunctionObject.  I'll make the code generic and add it to method_descriptor, classmethod_descriptor, and wrapper_descriptor.

Here's a patch that adds __text_signature__ support for three more builtin types:
  method_descriptor
  classmethod_descriptor
  wrapper_descriptor
  method-wrapper

The patch also modifies inspect.Signature so it recognizes these types.

Thanks, this is working here for the parameters. Is there a way to
specify the return annotation manually in the docstring?

Yes, it's just Python syntax, so you'd use "->".  However, you are not permitted to according to PEP 8:

"The Python standard library will not use function annotations as that would result in a premature commitment to a particular annotation style."

> Yes, it's just Python syntax, so you'd use "->".

I tried that, but it didn't filter through to inspect.signature().

> However, you are not permitted to according to PEP 8:

Ah, too bad. Return annotations are nice.

Larry,

Congrats on the amazing job you did with the arguments clinic.
And if you need any assistance with 'inspect.signature' I'd be glad to help.

Yury: Thanks!  I don't need any help right now though--just a review on this patch ;-)

Larry, just a small thing.. Could you please add something like "Parameter = cls._parameter_cls" in the "from_builtin" method? (see the discussion in #17373)

Okay, life has gotten even more complicated.

In another issue (#20172) Zachary Ware pointed out that Argument Clinic needs to generate "self" parameters in the text string.  But this complicates life for inspect.Signature, which needs to not publish the "self" parameter when it's been bound.  I'm busy hacking up clinic.py to fix this right now and hope to have a patch later today.

> But this complicates life for inspect.Signature, which needs to not publish the "self" parameter when it's been bound.

That's already supported, isn't it?

>>> str(inspect.signature(F.a))
'(self, a)'
>>> str(inspect.signature(F().a))
'(a)'

Not for builtins.

Another issue is that with the patch applied help() is broken for certain forms
of docstrings:

from decimal import *
>>> print(setcontext.__doc__)

setcontext(c) - Set a new default context.

>>> help(setcontext)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/stefan/hg/cpython/Lib/_sitebuiltins.py", line 99, in __call__
    return pydoc.help(*args, **kwds)
  File "/home/stefan/hg/cpython/Lib/pydoc.py", line 1792, in __call__
    self.help(request)
  File "/home/stefan/hg/cpython/Lib/pydoc.py", line 1842, in help
    else: doc(request, 'Help on %s:', output=self._output)
  File "/home/stefan/hg/cpython/Lib/pydoc.py", line 1578, in doc
    pager(render_doc(thing, title, forceload))
  File "/home/stefan/hg/cpython/Lib/pydoc.py", line 1571, in render_doc
    return title % desc + '\n\n' + renderer.document(object, name)
  File "/home/stefan/hg/cpython/Lib/pydoc.py", line 358, in document
    if inspect.isroutine(object): return self.docroutine(*args)
  File "/home/stefan/hg/cpython/Lib/pydoc.py", line 1323, in docroutine
    signature = inspect.signature(object)
  File "/home/stefan/hg/cpython/Lib/inspect.py", line 1551, in signature
    raise ValueError(msg)
ValueError: no signature found for builtin function <built-in function setcontext>

Perhaps this form of docstrings is discouraged (I used it because it looks nice
in pydoc), but nevertheless it might be present in third party modules.

Here's an updated patch.  I tried to "do it right" which wound up being a huge amount of work in Clinic.  The actual change to inspect.Signature was really easy, once I understood everything.

The churn in the .c files is because Clinic now uses the self converter's type for the parsing function, and (obviously) because it's now generating "self" in the signatures as appropriate.

Fun trivia: the "self" parameter to a builtin is always a positional-only parameter, even if all other argument processing for the function is PyArg_ParseTupleAndKeywords.  I think this patch marks the first time inspect.Signature will ever mark a parameter as positional-only!

> Another issue is that with the patch applied help() is broken for
> certain forms of docstrings:

Yeah.  We discussed this briefly in #19674.  I wanted to use a marker that wasn't The Convention That People Have Used For Decades but I felt overruled.  I want to revisit it for precisely the reason you cite.

(I just realized, Sphinx autodoc is irrelevant, as if the string is legitimate it be stripped off the docstring anyway.)

Dang it, I forgot to add the second patch.  Here it is.

Updated the patch.  (Diff #2 apparently didn't apply cleanly, so we didn't get a review link.)


Old-guard core devs: I'm *really* desperate for a review of this patch.  You don't have to review everything thing, just these files:

    * Include/object.h
    * Objects/descrobject.c
    * Objects/methodobject.c
    * Objects/typeobject.c

I can get a different reviewer for the other files.  But I worry about touching these tender bits of the type system and I want to make sure that

a) I haven't done something awful, and
b) I haven't missed something important.

Just that part of the diff is 345 lines, and it's pretty regular.  I'd be surprised if it took you a whole hour.

If you have any questions email me, I'd be thrilled to answer 'em if it means I can get this patch checked in.  Maintaining the patch is overhead that I just don't need during the Derby.

Nick, could you maybe review this?

Whoever does the review, could you post here?  I feel bad enough asking y'all, maybe we don't need multiple people doing it ;-)

Looking now.

Larry clarified that the signature(min) change in this patch was actually restoring the Python 3.3 behaviour, so I think with the addition of some relevant test cases to the test suite, go for it :)

Now looking.  Note that a few parts of the patch no longer cleanly apply:

hg import --no-c http://bugs.python.org/review/download/issue20189_10572.diff
applying http://bugs.python.org/review/download/issue20189_10572.diff
patching file Modules/_cursesmodule.c
Hunk #2 FAILED at 649
1 out of 2 hunks FAILED -- saving rejects to file Modules/_cursesmodule.c.rej
patching file Modules/_dbmmodule.c
Hunk #3 FAILED at 310
1 out of 5 hunks FAILED -- saving rejects to file Modules/_dbmmodule.c.rej
patching file Modules/_opcode.c
Hunk #2 FAILED at 63
1 out of 2 hunks FAILED -- saving rejects to file Modules/_opcode.c.rej
patching file Modules/zlibmodule.c
Hunk #2 FAILED at 226
1 out of 8 hunks FAILED -- saving rejects to file Modules/zlibmodule.c.rej
patching file Tools/clinic/clinic.py
Hunk #6 FAILED at 1482
Hunk #7 succeeded at 1502 with fuzz 2 (offset -14 lines).
Hunk #12 FAILED at 2393
2 out of 22 hunks FAILED -- saving rejects to file Tools/clinic/clinic.py.rej
abort: patch failed to apply

Updating / merging / resolving now, but it will take me a few minutes.

Here's a fresh patch against trunk.  It applies cleanly against current tip (725bc24f5492).

I limited myself to the four files you mentioned, and they look totally fine. Together with Nick's view you should have enough core review now, right?

I do.  Thanks for your time!

A few issues with this patch:

1) help(os) raises ValueError

"""
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "P:\ath\to\cpython\lib\_sitebuiltins.py", line 99, in __call__
    return pydoc.help(*args, **kwds)
  File "P:\ath\to\cpython\lib\pydoc.py", line 1792, in __call__
    self.help(request)
  File "P:\ath\to\cpython\lib\pydoc.py", line 1842, in help
    else: doc(request, 'Help on %s:', output=self._output)
  File "P:\ath\to\cpython\lib\pydoc.py", line 1578, in doc
    pager(render_doc(thing, title, forceload))
  File "P:\ath\to\cpython\lib\pydoc.py", line 1571, in render_doc
    return title % desc + '\n\n' + renderer.document(object, name)
  File "P:\ath\to\cpython\lib\pydoc.py", line 356, in document
    if inspect.ismodule(object): return self.docmodule(*args)
  File "P:\ath\to\cpython\lib\pydoc.py", line 1142, in docmodule
    contents.append(self.document(value, key, name))
  File "P:\ath\to\cpython\lib\pydoc.py", line 358, in document
    if inspect.isroutine(object): return self.docroutine(*args)
  File "P:\ath\to\cpython\lib\pydoc.py", line 1323, in docroutine
    signature = inspect.signature(object)
  File "P:\ath\to\cpython\lib\inspect.py", line 1551, in signature
    raise ValueError(msg)
ValueError: no signature found for builtin function <built-in function abort>
"""

2) help(pickle.dump) shows "module" as the first argument.  Of course, that's true in C, but not in Python.  This is the same for all module-level builtins.

"""
>>> help(pickle.dump)
Help on built-in function dump in module _pickle:

dump(module, obj, file, protocol=None, *, fix_imports=True)
    Write a pickled representation of obj to the open file object file.

<etc.>

>>> pickle.dump.__text_signature__
'(module, obj, file, protocol=None, *, fix_imports=True)'
"""

3) A module-level function with a positional-only parameter named 'module' in the first position causes an assertion error:

Clinic block:
"""
/*[clinic input]
_winapi.GetModuleFileName

    module: HMODULE
    /

Return the fully-qualified path for the file that contains module.

<etc>
[clinic start generated code]*/
"""

Traceback:
"""
P:\ath\to\cpython>PCbuild\python_d.exe Tools\clinic\clinic.py Modules\_winapi.c
Error in file "Modules\_winapi.c" on line 1299:
Exception raised during parsing:
Traceback (most recent call last):
  File "Tools\clinic\clinic.py", line 1099, in parse
    parser.parse(block)
  File "Tools\clinic\clinic.py", line 2283, in parse
    self.state(None)
  File "Tools\clinic\clinic.py", line 3022, in state_terminal
    self.function.docstring = self.format_docstring()
  File "Tools\clinic\clinic.py", line 2847, in format_docstring
    assert isinstance(parameters[0].converter, self_converter)
AssertionError
"""

Problems 1 (ValueError from help(os)) and 2 ('module' as first param) were simple fixes:

The attached patch fixes problem 1 (and is probably worth doing anyway, since inspect.signature has the ability to raise ValueError, and (IMO) help(<module>) should never raise an exception.

Problem 2 is just a matter of adding self.show_in_signature = False to the first param of module-level functions.  I've left a review comment at the right line.

Problem 3 would also be fixed by re-adding 'module' to c_keywords, but since you want it to work without that, that's out, and I'm not sure what the proper fix would be otherwise.

The current patch no longer applies cleanly after #20287; I would just post an updated version of the entire patch with my changes as well, but the merge is not trivial and I don't want to screw it up :)

Your fixes for #1 and #2 were fine, I've incorporated them into the patch.  I'll update the diff after I've added the tests Nick suggested.

The assertion failure in #3 will also be gone, replaced with a failure:

  You can't have two parameters named module!

The problem is that we silently inserted a self converter for the first argument, and because this is a module-level function, that "self" parameter is naturally named "module".

I have a fix in mind for this: basically to teach Argument Clinic that the parser function and impl function have different namespaces, and to map names in the first to the second.  So, you could have a parameter named "args", and Clinic would notice, and rename the variable in the parser function "args_value" or something, and then pass it in in the right spot.  Once I've done that, it'd be easy to make it also rename the secret self converter name to "_module" or something.  Anyway, long story short, let's not try to fix #3 in this patch.

Larry Hastings added the comment:
> Once I've done that, it'd be easy to make it also rename the secret self converter name to "_module" or something.  Anyway, long story short, let's not try to fix #3 in this patch.

That sounds fine.  _winapi is the only place I've seen that has a
'module' parameter and it's not finished yet anyway, so it can wait a
bit more (less than a week, though ;-).

Here is the hopefully-final patch for this issue.  I incorporated the suggested changes from Zachary Ware.  Also I fixed some "cls" parameters that were leaking into the signatures.  I think this is ready for checkin!

Argh.  I lost 1.5 day's worth of work on revision 6 of this patch last
night, due to me being tired and over-aggressively cleaning my working
directories.  I will have to reconstruct it from memory, hopefully
Tuesday.  (I basically know what I did, and going through the process
again should be much quicker.)

I have more C fixes by the way:

  * PyTypeObject needs to also expose __text_signature__.  Otherwise
    builtin classes can't have signatures.

  * There are a bunch of default docstrings for tp_ etc. slots
    (__add__, __call__) that live in typeobject.c.  I'll hand-convert
    them to have signatures and reasonable docstrings.

At last, my refreshed patch.  Changes from the previous patch:

* Had another mildly bright idea.  The name "PyTypeObject *cls"
  is a holdover from < Python 2.2 days, before the merging of classes
  and types.  Now they're both the same thing and the official name
  in Python is "type".  So the self_converter uses the word "type".

* Added __text_signature__ to PyType_Type.

* Removed __text_signature__ from PyMemberDescr_Type, as it's not
  callable.  (What was I thinking!)

* Hand-coded signatures for default docstrings for slots.

* Hand-coded signature for type and object.  (The one for type isn't
  correct, I know, we'll get there.)

* clinic.py now generates self/module/type parameters for most
  text signatures; these are then stripped off if the function is
  bound.

* Ensured that inspect.signature raises ValueError if it can't
  generate a signature for a callable object.

* Added unit tests.

* Suppress the null "self" parameter for METH_STATIC calls in the impl.

* If you have an empty docstring for __new__ or __init__, it suppresses
  the static variables completely.

* Functions with a self converter use the type of self for the
  parsing function.  (Except __new__ and __init__, which must conform
  to using PyObject *.)

Boy am I emotionally ready to check this thing in.

Ok, I found the source of the real issue alluded to in the misguided comment about the 'cls' -> 'type' change that I left on Rietveld.

I was under the impression that with that change, 'help(datetime.datetime.now)' would show a signature of 'now(type, tz=None)'.  In actual fact, 'str(inspect.signature(datetime.datetime.now))' (correctly) returns (tz=None), and that's what help (incorrectly) displays.  To properly match the help output of Python-implemented methods, pydoc will need to add in the 'self' or 'cls' parameter somehow.

However, I think that situation can be resolved in another issue in favor of getting this in, with the few issues I pointed out on Rietveld fixed.

I'm happy to resolve it before checking in the patch.
A small delta like that doesn't need a full-on review.

If people said "eww" then I'll back it out.  Nobody said "eww"
to the "PyModuleDef *module" change (see below), and I'm not
here to pick a fight.

But let's talk about it a little!

--

First, the name *is* visible in Python, if you examine the unbound
version.

    >>> str(inspect.signature(_datetime.datetime.__dict__['now']))
    `(<type>, tz=None)`
    >>> help(_datetime.datetime.__dict__['now'])
    # ... shows help, including <type> parameter in the signature

The angle-brackets are Signature's way of denoting a positional-only
parameter, which it is.   That parameter isn't addressable by name.

(The ugly angle brackets are being addressed in another issue.)

--

Second, I'm surprised at the behavior of help.  I hadn't realized
that it showed you bound parameters for callables you passed in.

    >>> class C:
    ...   @classmethod
    ...   def wife(cls, a, b):
    ...     print(cls, a, b)
    ... 
    >>> help(C.wife)

That shows "cls" as part of the signature.  But inspect.signature
does not:

    >>> str(inspect.signature(C.wife))
    '(a, b)'

FWIW help on a callable bound using functools.partial shows you
help on the functools.partial class, so no guidance there.

(help() only goes one level deep on this by the way.  If you have
a types.MethodType which binds another types.MethodType, help
only peeks in the first one.  But now I'm just showing off.)

Anyway, I think it's odd, but I'm not here to change the behavior
of help.  I'll work on fixing help so it shows the already-bound
parameters.

--

Third, it's inconvenient to use "type" as an identifier in Python code,
because "type" is a Python builtin.  And it's impossible to use "class"
because it's a keyword.  So people use "cls" or "klass" according to
personal taste.

We don't have these restrictions in C.  So actually "class" would
work--except C++ uses "class" as a keyword.  Let's not go there.
But "type" works fine.

--

Fourth, I already called the first parameter "type" for __new__
calls, as that seemed to be the convention there.  It was just
class methods where I called it "cls".  But... __new__ *is* a
class method.  This is an artificial distinction.

At the very least, I want Argument Clinic to use one name
consistently.  If everyone would prefer "cls" I don't care all
that much.  But I think "type" is a better name.  (For one,
it's not misspelled.)

--

Fifth, up until Argument Clinic, most callables had "PyObject *self"
as their first parameter.  But module-level callables never actually
get a "self", there's no "self" to call them with.  They actually
take the module object.  Everybody called the first parameter
"self" because they copied-and-pasted it from other code, and everyone
ignores that parameter anyway.

I proposed generating "PyModuleDef *module" there instead, Guido said
"good idea!"  I see this as similar, though the degree of error is
not as large.

A little more on consistency and inconsistency.

I count 109 tp_new callback functions in CPython, and they overwhelmingly call the first parameter "PyTypeObject *type" (93 instances).  In second place is "PyObject *self" (9 instances), which is flat-out wrong.

I count 21 METH_CLASS callback functions in CPython; they prefer calling the first parameter "PyObject *cls" (16 instances).  In second place is "PyTypeObject *type" (3 instances).

Both callbacks are class methods.  And both callbacks are passed the *exact same object* for their first parameter, the PyTypeObject * representing that type.

I can see no good reason why they should have different names in different callbacks.  There's no practical or semantic difference between the two.  I suspect it's something silly like legacy code / copying and pasting / force of habit, perhaps carried over from the days before type/class unification.

Note that tp_new is a static method, not a class method (the type creation
machinery takes care of passing in the right class rather than the
descriptor machinery)

> Note that tp_new is a static method, not a class method (the type
> creation machinery takes care of passing in the right class rather
> than the descriptor machinery)

I admit I didn't know that.

But from a practical perspective, surely you agree that tp_new walks and quacks like a class method?  That I, as an author of an extension type, should think of it as such?

It doesn't act like a class method, though, it acts like a static method:

    >>> int.__new__()
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    TypeError: int.__new__(): not enough arguments
    >>> int.__new__(int)
    0

You have to *write* __new__ and tp_new as if they were class methods (because the type machinery expects you to do so), but you have to *call* them like static methods if you're invoking them directly for some reason.

(Also, I can't give you a solid reason for *why* it's like that - Guido just wrote it that way, and the type machinery is hairy enough that I have no intentions of second guessing him on that one)

Oh, yes, now I remember - it *has* to be that way, otherwise upcalls from subclass __new__ methods don't do the right thing (int.__new__(MyInt), etc), just as you need to pass the current type in explicitly for cooperative super calls. This is perhaps *the* most obscure design detail of the type system that I'm aware of - I have to go scratching around in my brain for the reason every time it comes up, which is fortunately almost never :)

Okay, one more diff.  I have high hopes for this, but then I had high hopes yesterday.

Nick, could you review the PyTypeObject changes in this patch?  Obviously I'd love a review of the whole thing, but if you can only make a little time, the crucial part is the "delta from patch set" 5 for typeobject.c.

First thing: I must never have run the unit test suite before cutting the diff yesterday, because I did today and there were a bunch of problems.  That's clowny and I apologize.  But it's fixed now, and I assure you, there's no way I would have actually checked this in without running the test suite immediately before.

Here's what changed today:

Core:
* Modified typeobject.c so that when creating an object,
  if it copies the type's "__doc__", it skips past
  the signature, because type objects are callable
  and have signatures now.

* Default value for dict.fromkeys parameter is now None.
  (Previously it was NULL, which was simply wrong.)

Lib and tests:

* pydoc now catches both ValueError and TypeError when it tries
  inspect.signature.

* Added a fix for unittest.mock courtesy of Michael Foord.
  It previously assumed that anything it could get an
  inspect.Signature for was written in Python.  Now that
  that's no longer true, it broke some other code.  Michael's
  pretty confident that's the right fix, and in any case it
  makes the tests pass again.

* Bashed up some IDLE unit tests that depend on docstrings.  These are
  accurate for now, but look wrong because of the __text_signature__
  grabbing the first line when it shouldn't.  When I get to put in
  the new signature syntax, these will break again and I'll put them
  back.

* General unit test fixes, to live in this modern world.

Tools:

* Argument Clinic now makes sure that parser functions for __new__
  are always of type newfunc, the type of the tp_new slot.
  Similarly, parser functions for __init__ are now always of
  type initproc, the type of tp_init.

Scanned the whole patch, especially the type changes. This looks like a solid approach to me.

For 3.5, PEP 457 might want to consider proposing a tp_sig slot and splitting the signature out at type creation time rather than on attribute lookup. The current dynamic approach is fine for 3.4, though.

Okay, I'm checking this beast in.  Hooray!  Thanks for your reviews, everybody!

--

I thought it was still possible to introduce objects into Python at runtime without calling PyType_Ready on their type.  If that's true,
then there wouldn't necessarily *be* a type creation time at which
we could do the signature splitting.

Is that no longer allowed as of 3.4?  Are all types required to be
registered with PyType_Ready before objects of that type are introduced into the Python runtime?  If so, hooplah!

There are probably still ways to do it, but we don't *support* doing it (and I'm pretty sure we've fixed them all in the builtins and stdlib).

However, yes, that's another good reason to be conservative in only doing the split into signature+doc at attribute lookup time.

I just realized, I forgot to fix the bug Zach reported, where help(bound_thing) should still show the class or self parameter.
I'm going to check this in anyway, and file a fresh bug on myself to
address that.

New changeset 85710aa396ef by Larry Hastings in branch 'default':
Issue #20189: Four additional builtin types (PyTypeObject,
http://hg.python.org/cpython/rev/85710aa396ef

Phew!  Thanks again, everybody!