classification
Title: help(open) eats first line
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.4
process
Status: closed Resolution: out of date
Dependencies: 20189 20326 Superseder:
Assigned To: zach.ware Nosy List: gennad, larry, meador.inge, serhiy.storchaka, skrah, vajrasky, yselivanov, zach.ware
Priority: critical Keywords: 3.3regression, patch

Created on 2013-12-26 20:35 by serhiy.storchaka, last changed 2014-01-28 15:47 by larry. This issue is now closed.

Files
File name Uploaded Description Edit
20075.patch gennad, 2013-12-27 00:21 review
20075-2.patch gennad, 2013-12-27 18:57 review
20075-3.patch gennad, 2014-01-01 15:24 review
Messages (20)
msg206962 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-26 20:35
The output of help(open) (and `pydoc open`) in 3.4 is:

Help on built-in function open in module io:

open(...)
    errors=None, newline=None, closefd=True, opener=None) -> file object
    
    Open file and return a stream.  Raise IOError upon failure.
...

In 3.3 and older it works correctly:

Help on built-in function open in module io:

open(...)
    open(file, mode='r', buffering=-1, encoding=None,
         errors=None, newline=None, closefd=True, opener=None) -> file object
    
    Open file and return a stream.  Raise IOError upon failure.
...
msg206965 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-12-26 21:32
Interestingly, it doesn't look like pydoc's fault:

P:\ath\to\cpython\>PCbuild\python_d.exe -ISc "import sys;print(sys.version);print(open.__doc__[:75]);print('pydoc' in sys.modules)"
3.4.0b1 (default:94a04b8b3a12, Dec 26 2013, 09:27:14) [MSC v.1600 32 bit (Intel)]
     errors=None, newline=None, closefd=True, opener=None) -> file object


False
msg206968 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-26 21:47
Indeed. However in Modules/_io/_iomodule.c:

PyDoc_STRVAR(open_doc,
"open(file, mode='r', buffering=-1, encoding=None,\n"
"     errors=None, newline=None, closefd=True, opener=None) -> file object\n"
"\n"
"Open file and return a stream.  Raise IOError upon failure.\n"
...

Perhaps Larry has relations to this.
msg206969 - (view) Author: Meador Inge (meador.inge) * (Python committer) Date: 2013-12-26 22:01
Looks like the changes from 78ec18f5cb45 attempt to skip the signature,
but can't handle multi-line signatures.
msg206975 - (view) Author: Gennadiy Zlobin (gennad) * Date: 2013-12-27 00:21
Hi guys,

probably this patch can fix it?
msg206984 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-27 07:55
LGTM.
msg206985 - (view) Author: Vajrasky Kok (vajrasky) * Date: 2013-12-27 08:13
The patch does not fix it. It becomes like this:

open(...)
    Open file and return a stream.  Raise IOError upon failure.

It's not just help(open) has problem, help(sqlite3.connect) got it as well:

connect(...)
    check_same_thread, factory, cached_statements, uri])
    
    Opens a connection to the SQLite database file *database*. You can use
msg206989 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-12-27 10:35
The best fix would be to convert the docstrings to something inspect can parse.  Preferably by converting the functions to use Argument Clinic, though you could manually mark up the docstring by hand if necessary.
msg206992 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-12-27 13:05
> The best fix would be to convert the docstrings to something inspect can
> parse.  Preferably by converting the functions to use Argument Clinic,
> though you could manually mark up the docstring by hand if necessary.

We can't check all docstrings in the stdlib and in all third-party libraries.
msg206994 - (view) Author: Gennadiy Zlobin (gennad) * Date: 2013-12-27 14:12
Yes, so basically signature line in help(open) is not shown because ast.parse fails to parse the return value

-> file object

According to grammar, it should be 
-> file 
or 
-> 'file object' 
or something like this.

as for sqlite, it fails to parse square brackets:

connect(database[, timeout, detect_types, isolation_level,\n\
        check_same_thread, factory, cached_statements, uri])

As an idea, maybe we can come up with a failover i.e. if ast can't parse the signature, just use __text_signature__ instead of signature object:

Lib/pydoc.py:1325
if not argspec:
-   argspec = '(...)'
+   argspec = object.__text_signature__


Or probably just don't show the signature if it is not formatted correctly as it is now (after the patch applied).
msg206996 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2013-12-27 15:41
The patch looks good to me (aside from extra whitespace on the blank lines in methodobject.c, and I agree with Serhiy about s/brackets/parens/).  Also, I like the suggestion of using __text_signature__ instead of '(...)'.  However, just to avoid any possible issues with __text_signature__ being blank or missing, I would go with `argspec = getattr(object, '__text_signature__', '') or '(...)'` instead of straight `object.__text_signature__` (and note that there are two places to change in pydoc).
msg207004 - (view) Author: Gennadiy Zlobin (gennad) * Date: 2013-12-27 17:08
Thank you for the comments! I'll update the patch.


BTW is it safe to update Lib/inspect.py:2004 ?

- return cls(parameters, return_annotation=cls.empty)
+ return cls(parameters, return_annotation=f.returns.s or cls.empty)

Looks like the return value is not shown in signature (if it parsed correctly) because currently we explicitly pass cls.empty instance, but if we'd pass f.returns.s, the return value is shown.

Or it is correct behavior?
msg207007 - (view) Author: Gennadiy Zlobin (gennad) * Date: 2013-12-27 18:57
So, looks like it works for me and all tests pass.
Here's a new patch.
Feel free to revert Lib/inspect.py:2004-2009 if this is incorrect behavior.
msg207008 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2013-12-27 19:11
One of the relevant PEPs (PEP 8?  PEP 7?  the annotations PEP?) states that the Python standard library is not permitted to use annotations.  And considering that Argument Clinic is an internal-only tool, we could probably justify the decision to not allow annotations to creep through.

That said, I think it's harmless, and it might be useful to somebody, so go ahead and propagate the annotation from the __text_signature__ into inspect.Signature if we get a valid one.  But please create a separate issue for it.  (I encourage you to cut-and-paste this text into the description of that new issue.)
msg207014 - (view) Author: Gennadiy Zlobin (gennad) * Date: 2013-12-27 23:25
I'm sorry, I'm not sure I caught the idea. So, I need to create an issue with description

"propagate the annotation from the __text_signature__ into inspect.Signature if we get a valid one." ?
msg207134 - (view) Author: Gennadiy Zlobin (gennad) * Date: 2014-01-01 15:24
Zachary, thank you for review. Here's the updated patch.
msg208552 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-01-20 17:02
Gennadiy, sorry this stalled out like it did.

The patched function will be moving as part of issue20189.  Larry, if you want to incorporate this patch into that patch, please do so; otherwise I'll get this one updated and committed as soon after you commit that one as I can.  Gennadiy, if you beat me to updating this patch after Larry commits #20189, please do so.

(Setting priority as 'critical' since it would be quite embarrassing to release 3.4.0 with broken "help(open)".)
msg208684 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2014-01-21 18:52
See also #20326.
msg209558 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2014-01-28 15:45
With #20326 fixed, this is no longer an issue.

Gennadiy, thank you for the patch, and I'm sorry it ended up being unused.
msg209559 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-01-28 15:47
Yup, sorry about that.  We're moving pretty fast with the Derby right now, and sometimes we don't figure out something important until later, and sometimes that means wasted effort.  Sorry!
History
Date User Action Args
2014-01-28 15:47:29larrysetmessages: + msg209559
2014-01-28 15:45:44zach.waresetstatus: open -> closed
resolution: out of date
messages: + msg209558

stage: commit review -> resolved
2014-01-25 08:09:08ncoghlansetdependencies: + Argument Clinic should use a non-error-prone syntax to mark text signatures
2014-01-23 18:13:52serhiy.storchakasetnosy: + yselivanov
2014-01-21 18:52:31skrahsetnosy: + skrah
messages: + msg208684
2014-01-21 18:46:07serhiy.storchakasetassignee: zach.ware
2014-01-20 17:02:49zach.waresetpriority: normal -> critical

dependencies: + inspect.Signature doesn't recognize all builtin types
messages: + msg208552
2014-01-01 15:24:33gennadsetfiles: + 20075-3.patch

messages: + msg207134
2013-12-27 23:25:28gennadsetmessages: + msg207014
2013-12-27 19:11:25larrysetmessages: + msg207008
2013-12-27 18:57:13gennadsetfiles: + 20075-2.patch

messages: + msg207007
2013-12-27 17:08:13gennadsetmessages: + msg207004
2013-12-27 15:41:40zach.waresetmessages: + msg206996
2013-12-27 14:12:38gennadsetmessages: + msg206994
2013-12-27 13:05:45serhiy.storchakasetmessages: + msg206992
2013-12-27 10:35:00larrysetmessages: + msg206989
2013-12-27 08:13:36vajraskysetnosy: + vajrasky
messages: + msg206985
2013-12-27 07:55:03serhiy.storchakasetmessages: + msg206984
stage: commit review
2013-12-27 00:21:10gennadsetfiles: + 20075.patch

nosy: + gennad
messages: + msg206975

keywords: + patch
2013-12-26 22:01:32meador.ingesetnosy: + meador.inge
messages: + msg206969
2013-12-26 21:47:40serhiy.storchakasetnosy: + larry
messages: + msg206968
2013-12-26 21:32:56zach.waresetnosy: + zach.ware
messages: + msg206965
2013-12-26 20:35:20serhiy.storchakacreate