msg211768 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 00:19 |
inspect has a bug:
* inspect.ismethod() returns False for bound methods on builtins.
If I fix that, that exposes a bug in pydoc:
* pydoc's two docroutine() functions assume that bound methods
have a __func__; bound builtins do not.
The only reason pydoc assumed the __func__ attribute was so it could use that instead of the bound function when getting the signature. I don't know why it cared, but: when it does so, that means it displays "self" for bound methods implemented in Python.
However, since it doesn't do that for bound methods implemented in C, now the behavior is inconsistent. Should it display self or not? The consensus was, it should not. This would make pydoc consistent with inspect.signature.
This patch therefore changes a third thing:
* pydoc: don't display "self" for bound methods implemented in Python.
I propose to merge this for 3.4.0rc2.
|
msg211769 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 00:23 |
Larry, I think you can use undocumented and private (but still heavily tested) 'inspect._signature_internal(skip_bound_arg=False)', instead of signature. That will give you a Signature object with 'self' parameter always included.
And in 3.5 we'll probably make this option (renamed, hopefully), public, and switch pydoc to use new API again.
What do you think?
|
msg211770 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 00:25 |
(The thing with my proposal, is that pydoc will handle everything that 'inspect.signature' supports + it's behaviour won't change at all)
|
msg211771 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 00:29 |
Whoops, the test suite hadn't finished when I posted that, and it had an error. Here's an updated patch that fixes the tests.
Also, there's one more minor fix in the patch. pydoc has cut-and-pasted code for HTML versus text rendering, and they had fallen out of sync. This patch brings them into sync.
|
msg211772 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 00:36 |
I don't think we should touch 'inspect.isbuiltin' at this stage.
Please consider my proposal instead.
|
msg211773 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 00:37 |
We discussed it in python-dev:
https://mail.python.org/pipermail/python-dev/2014-January/132051.html
And the overwhelming majority voted +1 for "don't show self for bound methods". So I'd like to change it for 3.4. (I meant to get it in earlier but lacked the time.)
|
msg211774 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 00:38 |
I'm not proposing to modify isbuiltin. Rather, I'm proposing to fix a bug in ismethod.
|
msg211775 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 00:41 |
> Rather, I'm proposing to fix a bug in ismethod.
Oh, sorry, that's what I meant. I don't think it's good to fix it either.
But please hold on, I think I found a bug in inspect.signature.
|
msg211777 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 00:54 |
Indeed, here it is: #20711
|
msg211778 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 00:58 |
So why is it necessary (and a release blocker) to fix a bug in inspect._signature_fromstr, but a terrible idea to fix a bug in inspect.ismethod?
|
msg211779 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 01:06 |
> So why is it necessary (and a release blocker) to fix a bug in inspect._signature_fromstr, but a terrible idea to fix a bug in inspect.ismethod?
I was actually writing a comment, when I received this on my email ;) Let me explain my point of view.
> And the overwhelming majority voted +1 for "don't show self for bound methods". So I'd like to change it for 3.4.
Sorry, I didn't know about it. I lost track of the relevant issue, and thought that we still need to keep the old behaviour in tact.
> inspect.ismethod() returns False for bound methods on builtins.
I'm +1 in principle, the only thing that bugs me, is that 'ismethod' is probably 100x more popular API then signature, and event getfullargspec. I'm just not sure what are the consequences. Maybe, it's possible to fix pydoc without touching 'ismethod'? My 2 cents.
Now, about the newly discovered bug in getfullargspec() [and _signature_fromstr]. I put a "release blocker" on that issue because it's a new change, that 'getfullargspec' now uses 'inspect.signature', and I don't think we should ship this new feature broken. But certainly, it's up to you to let this in 3.4.0.
|
msg211782 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 01:17 |
Okay, that's a fair point. I checked, and the documentation specifically says that ismethod only returns true on bound methods implemented in Python. I think that's a bad API, it should be agnostic about the implementation language. But I'll remove the change to ismethod and rework the patch to pydoc around it.
|
msg211784 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 01:25 |
> I think that's a bad API, it should be agnostic about the implementation language.
Agree.
Re #20711: Please take a look at the patch I wrote. It's your code I modified, you know it and the __text_signature__ quirks better than anyone.
|
msg211785 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 01:28 |
Here's a revised patch that doesn't modify inspect.
|
msg211786 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 01:51 |
A slight tweak to the patch. Previously I was just using truth testing on the value I got from "__self__", but that's wrong if the object is considered false (e.g. ''.zfill). (Yury got this right in #20711, and I copied from him!)
|
msg211794 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 03:11 |
An even slighter tweak to the patch, just using De Morgan's law to make the code easier to read.
|
msg211811 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 06:52 |
Updated with tests. And I built without docstrings and it still passed. And if I revert the change to Lib/pydoc.py the tests fail.
|
msg211813 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 07:13 |
I went ahead and added tests for os.stat, and the unbound versions of the two class methods. Total of five new tests now.
|
msg211814 - (view) |
Author: Larry Hastings (larry) * |
Date: 2014-02-21 07:20 |
You're right, that was the exact same test ;-)
So what I did was: move the other tests down into that test class, as that's a better fit, and keep the new version of the test.
|
msg211815 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 07:31 |
Looks good to me. Those unit tests give some confidence ;)
|
msg211816 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-02-21 07:35 |
New changeset 5e73bb72662e by Larry Hastings in branch 'default':
Issue #20710: The pydoc summary line no longer displays the "self" parameter
http://hg.python.org/cpython/rev/5e73bb72662e
|
msg211818 - (view) |
Author: Yury Selivanov (yselivanov) * |
Date: 2014-02-21 07:43 |
With this patch 'help' now correctly renders classmethods, yay
|
msg213798 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-03-17 06:30 |
New changeset b2ee3fe195e2 by Larry Hastings in branch '3.4':
Issue #20710: The pydoc summary line no longer displays the "self" parameter
http://hg.python.org/cpython/rev/b2ee3fe195e2
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:59 | admin | set | github: 64909 |
2014-03-17 06:30:44 | python-dev | set | messages:
+ msg213798 |
2014-02-21 07:43:18 | yselivanov | set | messages:
+ msg211818 |
2014-02-21 07:37:16 | larry | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
2014-02-21 07:35:01 | python-dev | set | nosy:
+ python-dev messages:
+ msg211816
|
2014-02-21 07:31:55 | yselivanov | set | messages:
+ msg211815 |
2014-02-21 07:20:16 | larry | set | files:
+ larry.fix.help.on.bound.methods.8.diff
messages:
+ msg211814 |
2014-02-21 07:13:37 | larry | set | files:
+ larry.fix.help.on.bound.methods.7.diff
messages:
+ msg211813 |
2014-02-21 06:52:29 | larry | set | files:
+ larry.fix.help.on.bound.methods.6.diff
messages:
+ msg211811 |
2014-02-21 03:11:52 | larry | set | files:
+ larry.fix.help.on.bound.methods.5.diff
messages:
+ msg211794 |
2014-02-21 01:51:42 | larry | set | files:
+ larry.fix.help.on.bound.methods.4.diff
messages:
+ msg211786 |
2014-02-21 01:28:32 | larry | set | files:
+ larry.fix.help.on.bound.methods.3.diff |
2014-02-21 01:28:24 | larry | set | messages:
+ msg211785 |
2014-02-21 01:25:19 | yselivanov | set | messages:
+ msg211784 |
2014-02-21 01:17:02 | larry | set | messages:
+ msg211782 title: Make inspect and pydoc consistent about bound methods -> Make pydoc consistent about bound methods |
2014-02-21 01:06:47 | yselivanov | set | messages:
+ msg211779 |
2014-02-21 00:58:46 | larry | set | messages:
+ msg211778 |
2014-02-21 00:54:47 | yselivanov | set | messages:
+ msg211777 |
2014-02-21 00:41:10 | yselivanov | set | messages:
+ msg211775 |
2014-02-21 00:38:13 | larry | set | messages:
+ msg211774 |
2014-02-21 00:37:15 | larry | set | messages:
+ msg211773 |
2014-02-21 00:36:54 | yselivanov | set | messages:
+ msg211772 |
2014-02-21 00:29:59 | larry | set | files:
+ larry.fix.help.on.bound.methods.2.diff
messages:
+ msg211771 |
2014-02-21 00:25:41 | yselivanov | set | messages:
+ msg211770 |
2014-02-21 00:23:29 | yselivanov | set | messages:
+ msg211769 |
2014-02-21 00:19:41 | larry | set | type: behavior stage: patch review |
2014-02-21 00:19:31 | larry | create | |