msg210380 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-06 14:08 |
Following a new discussion of the format of the text signature, I decided to go with this:
<name-of-function>(...)\n--\n\n
See here for the discussion:
https://mail.python.org/pipermail/python-dev/2014-February/132271.html
Patch attached implementing this approach. Can I get a review? I want this in before tagging for rc1 on Saturday.
Other changes:
* clinic.py now generates the "/" marker in sigantures to denote
positional-only parameters.
* clinic.py --make is a lot faster; it prescans the file for any
clinic block signatures, and if it doesn't see any it skips the file.
* When generating the docstring for a function with optional groups,
the signature generated is not intended to be machine-readable.
So it omits the "$", the "/", and the "--" markers. (See
Modules/_cursesmodule.c for the one and only example.)
|
msg210382 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-06 14:15 |
Whoops, forgot to fix all the breakage in clinic_test.py. It's good now.
|
msg210387 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-06 14:35 |
> Whoops, forgot to fix all the breakage in clinic_test.py. It's good now.
Ah, you re-uploaded the patch and reset my code review comments. I'll repeat them here, just in case.
Reviewed the patch.
- In one of the C files I saw this signature: "(/)" -- probably should be just "()"
- signature for 'type' builtin was '(object_or_name, bases, dict)', which is a correct signature (minus the groups/optional parameters). In this patch it's a ValueError. Can we restore the prior behaviour? If not, then test_signature_on_class_without_init is incorrect now, as it should throw a ValueError for metaclasses.
- and some other minor stuff
|
msg210438 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-07 06:44 |
Thanks for noticing the "(/)", that's fixed.
Yes, the signature for type() was wrong. type() can accept either one parameter or three parameters--in other words, it uses optional groups. And we can't represent optional groups in an inspect.Signature signature in 3.4. And I assert that it's better to not have a signature than to have a wrong signature. So I've removed it.
I'm interested in your "other minor stuff". I'll try and leave the patch alone this time :)
If you apply diff #2, you'll have a failure in test_inspect. This is because there's a bug in inspect.Signature that it would take too long to fix, and I have to leave right now-ish. I changed the tests so they expect the correct results, but they get the wrong results at the moment.
Hopefully we can fix this really quickly.
--
Yuri: This change uncovered a lurking bug in inspect.Signature that I'm hoping you can fix. I could probably figure it out given enough time but I won't have time to look at it for about 24 hours.
Now that type() doesn't have a signature, I have discovered that some of the logic in Signature is wrong.
class C(type): pass
print(str(inspect.signature(C)))
The signature of that *should* be the same as type(). But inspect.signature() reports the signature as '()'.
The reason this happens: inspect.Signature gets to the "for base in mro" case for handling classes. The first base it tries is type(), but type() doesn't have a public signature so it keeps going. The next class in the MRO is object(), which has a signature of "()", so it uses that.
It shouldn't keep going! I'm 99% certain that the first entry in the MRO will always be callable. (Is it possible to have a type in Python that isn't callable?)
I *think* what it should do is: simply try the first entry in the MRO. If that has a signature, return it. If it doesn't have a signature, inspect.Signature should raise ValueError.
I also *think* that all the code after this comment:
# No '__text_signature__' was found for the 'obj' class.
should be removed. If C doesn't define its own __new__ or __init__, and its base class doesn't have __call__, then the signature of C *is* the signature of its base class. If inspect.Signature can't read that signature, then it can't return a signature for C.
|
msg210502 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2014-02-07 17:48 |
New patch still gives an assertion failure at line 153 of typeobject.c when trying to get __text_signature__ from anything that should have one (on Windows, at least).
Python 3.4.0b3+ (default, Feb 7 2014, 11:43:04) [MSC v.1600 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> type.__text_signature__
>>> object.__text_signature__
Assertion failed: *end == ')', file ..\Objects\typeobject.c, line 153
|
msg210524 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-07 18:55 |
Larry,
I'm attaching a revised version of the patch -- larry.even.newerer.signature.syntax.3.diff.
Changes:
1. test_signature_on_class_without_init was fixed,
inspect.signature was fixed as well. Now, for
class Meta(type): pass
"inspect.signature(Meta)" will raise a ValueError.
> I also *think* that all the code after this comment:
> # No '__text_signature__' was found for the 'obj' class.
> should be removed.
You are right, but I'd prefer to leave that code until 3.5, when we have correct signature for 'type'.
> The reason this happens: inspect.Signature gets to
> the "for base in mro" case for handling classes.
> The first base it tries is type(), but type() doesn't
> have a public signature so it keeps going. The next
> class in the MRO is object(), which has a signature
> of "()", so it uses that.
> It shouldn't keep going! I'm 99% certain that the first
> entry in the MRO will always be callable. (Is it possible
> to have a type in Python that isn't callable?)
Unfortunately, no, we need to traverse the MRO. See the
"test_signature_on_builtin_class" unit test, for instance.
The way I solved the current bug, is to fixing the code from
traversing the full MRO, to traversing the MRO without the
last item -- 'object' builtin.
2. _strip_non_python_syntax -> _signature_strip_non_python_syntax
Minor fixup, just to have all signature helper functions
in inspect.py follow one notation.
3. test_strip_non_python_syntax -> was moved to another
test case -- TestSignaturePrivateHelpers
4. I also fixed one thing that Zachary found: unused 's' variable
in the '_signature_fromstr' method.
Here's a quick summary of changes (diff of the diffs,
omitting some changes in tests):
https://gist.github.com/1st1/8869242
|
msg210529 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-07 19:10 |
One more patch (v4)
I fixed the commented out 'ThisWorksNow' test.
And also 'inspect.signature' now raises a bit
more coherent exception for that particular case.
|
msg210716 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2014-02-09 03:56 |
Updated patch fixes an assertion failure in typeobject.c (end doesn't point at the ")", it points at the newline immediately after it).
|
msg210719 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-09 04:27 |
Nick: I made the same fix on the plane, except I also added an assert that end - start >= 2. ;-)
I'm going to go through everyone's feedback, and if there are any significant changes I'll post a new patch, otherwise I think this patch is basically ready.
|
msg210723 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-02-09 06:16 |
New changeset 29d9638bf449 by Larry Hastings in branch 'default':
Issue #20530: Argument Clinic's signature format has been revised again.
http://hg.python.org/cpython/rev/29d9638bf449
|
msg210724 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-09 06:23 |
I made a couple final tweaks to what was essentially Nick's patch:
* Argument Clinic wraps parameters in the signature at 72 columns
instead of 79 columns. There are a couple extra characters that
get emitted, so this ensures that the generated lines are never > 80.
* Added an assert to typeobject.c that end - start > 2, so that
the subsequent assert on end[-1] is always a legal memory access.
* inspect._strip_non_python_syntax() now emits a space after commas in
the signature, enhancing readability on the off chance that someone
needs to read them.
* Added a couple extra tests for _strip_non_python_syntax().
Thanks for all the reviews and the fixes, everybody!
|
msg210799 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-02-10 06:23 |
New changeset 28aef6e22736 by Larry Hastings in branch 'default':
Issue #20530: The signatures for slot builtins have been updated
http://hg.python.org/cpython/rev/28aef6e22736
|
msg231584 - (view) |
Author: Jesús Cea Avión (jcea) * |
Date: 2014-11-24 00:48 |
Preparing a presentation about Python Magic methods I found something weird: (Python 3.4)
"""
>>> help(int.__lt__)
Help on wrapper_descriptor:
__lt__(self, value, /) <- THIS!!
Return self<value.
"""
I am amused about the "/)" suffix in the signature. It happens to all magic methods.
|
msg231758 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-11-27 12:33 |
Actually this is the wrong issue for that observation. You want the issue where yselivanov added emitting the "/" to pydoc.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:58 | admin | set | github: 64729 |
2014-11-27 12:33:20 | larry | set | messages:
+ msg231758 |
2014-11-24 00:48:14 | jcea | set | nosy:
+ jcea messages:
+ msg231584
|
2014-02-10 06:23:21 | python-dev | set | messages:
+ msg210799 |
2014-02-09 06:23:03 | larry | set | status: open -> closed resolution: fixed messages:
+ msg210724
stage: patch review -> resolved |
2014-02-09 06:16:10 | python-dev | set | nosy:
+ python-dev messages:
+ msg210723
|
2014-02-09 04:27:45 | larry | set | messages:
+ msg210719 |
2014-02-09 03:56:54 | ncoghlan | set | files:
+ issue20530_fixed_assertion.diff
messages:
+ msg210716 |
2014-02-07 19:10:52 | yselivanov | set | files:
+ larry.even.newerer.signature.syntax.4.diff
messages:
+ msg210529 |
2014-02-07 18:55:03 | yselivanov | set | files:
+ larry.even.newerer.signature.syntax.3.diff
messages:
+ msg210524 |
2014-02-07 17:48:27 | zach.ware | set | messages:
+ msg210502 |
2014-02-07 06:44:58 | larry | set | files:
+ larry.even.newerer.signature.syntax.2.diff
messages:
+ msg210438 |
2014-02-06 14:35:08 | yselivanov | set | messages:
+ msg210387 |
2014-02-06 14:15:17 | larry | set | files:
+ larry.even.newerer.signature.syntax.1.diff
messages:
+ msg210382 |
2014-02-06 14:14:40 | larry | set | files:
- larry.even.newerer.signature.syntax.1.diff |
2014-02-06 14:09:10 | larry | create | |