Make pydoc consistent about bound methods

[larryhastings]

BPO	20710
Nosy	@ncoghlan, @larryhastings, @1st1
Files	larry.fix.help.on.bound.methods.1.diff larry.fix.help.on.bound.methods.2.diff larry.fix.help.on.bound.methods.3.diff larry.fix.help.on.bound.methods.4.diff larry.fix.help.on.bound.methods.5.diff larry.fix.help.on.bound.methods.6.diff larry.fix.help.on.bound.methods.7.diff larry.fix.help.on.bound.methods.8.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/larryhastings'
closed_at = <Date 2014-02-21.07:37:16.068>
created_at = <Date 2014-02-21.00:19:31.215>
labels = ['type-bug', 'library', 'release-blocker']
title = 'Make pydoc consistent about bound methods'
updated_at = <Date 2014-03-17.06:30:44.023>
user = 'https://github.com/larryhastings'

bugs.python.org fields:

activity = <Date 2014-03-17.06:30:44.023>
actor = 'python-dev'
assignee = 'larry'
closed = True
closed_date = <Date 2014-02-21.07:37:16.068>
closer = 'larry'
components = ['Library (Lib)']
creation = <Date 2014-02-21.00:19:31.215>
creator = 'larry'
dependencies = []
files = ['34162', '34163', '34165', '34166', '34168', '34170', '34171', '34172']
hgrepos = []
issue_num = 20710
keywords = ['patch']
message_count = 23.0
messages = ['211768', '211769', '211770', '211771', '211772', '211773', '211774', '211775', '211777', '211778', '211779', '211782', '211784', '211785', '211786', '211794', '211811', '211813', '211814', '211815', '211816', '211818', '213798']
nosy_count = 4.0
nosy_names = ['ncoghlan', 'larry', 'python-dev', 'yselivanov']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue20710'
versions = ['Python 3.4']

[larryhastings]

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.

[1st1]

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?

[1st1]

(The thing with my proposal, is that pydoc will handle everything that 'inspect.signature' supports + it's behaviour won't change at all)

[larryhastings]

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.

[1st1]

I don't think we should touch 'inspect.isbuiltin' at this stage.
Please consider my proposal instead.

[larryhastings]

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.)

[larryhastings]

I'm not proposing to modify isbuiltin. Rather, I'm proposing to fix a bug in ismethod.

[1st1]

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.

[1st1]

Indeed, here it is: bpo-20711

[larryhastings]

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?

[1st1]

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.

[larryhastings]

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.

[1st1]

I think that's a bad API, it should be agnostic about the implementation language.

Agree.

Re bpo-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.

[larryhastings]

Here's a revised patch that doesn't modify inspect.

[larryhastings]

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 bpo-20711, and I copied from him!)

[larryhastings]

An even slighter tweak to the patch, just using De Morgan's law to make the code easier to read.

[larryhastings]

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.

[larryhastings]

I went ahead and added tests for os.stat, and the unbound versions of the two class methods. Total of five new tests now.

[larryhastings]

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.

[1st1]

Looks good to me. Those unit tests give some confidence ;)

[python-dev]

New changeset 5e73bb72662e by Larry Hastings in branch 'default':
Issue bpo-20710: The pydoc summary line no longer displays the "self" parameter
http://hg.python.org/cpython/rev/5e73bb72662e

[1st1]

With this patch 'help' now correctly renders classmethods, yay

[python-dev]

New changeset b2ee3fe195e2 by Larry Hastings in branch '3.4':
Issue bpo-20710: The pydoc summary line no longer displays the "self" parameter
http://hg.python.org/cpython/rev/b2ee3fe195e2