Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

help(open) eats first line #64274

Closed
serhiy-storchaka opened this issue Dec 26, 2013 · 20 comments
Closed

help(open) eats first line #64274

serhiy-storchaka opened this issue Dec 26, 2013 · 20 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

BPO 20075
Nosy @larryhastings, @skrah, @meadori, @zware, @serhiy-storchaka, @1st1, @vajrasky
Dependencies
  • bpo-20189: inspect.Signature doesn't recognize all builtin types
  • bpo-20326: Argument Clinic should use a non-error-prone syntax to mark text signatures
  • Files
  • 20075.patch
  • 20075-2.patch
  • 20075-3.patch
  • 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/zware'
    closed_at = <Date 2014-01-28.15:45:44.749>
    created_at = <Date 2013-12-26.20:35:20.849>
    labels = ['interpreter-core', 'type-bug']
    title = 'help(open) eats first line'
    updated_at = <Date 2014-01-28.15:47:29.589>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2014-01-28.15:47:29.589>
    actor = 'larry'
    assignee = 'zach.ware'
    closed = True
    closed_date = <Date 2014-01-28.15:45:44.749>
    closer = 'zach.ware'
    components = ['Interpreter Core']
    creation = <Date 2013-12-26.20:35:20.849>
    creator = 'serhiy.storchaka'
    dependencies = ['20189', '20326']
    files = ['33276', '33280', '33292']
    hgrepos = []
    issue_num = 20075
    keywords = ['patch', '3.3regression']
    message_count = 20.0
    messages = ['206962', '206965', '206968', '206969', '206975', '206984', '206985', '206989', '206992', '206994', '206996', '207004', '207007', '207008', '207014', '207134', '208552', '208684', '209558', '209559']
    nosy_count = 8.0
    nosy_names = ['larry', 'skrah', 'meador.inge', 'gennad', 'zach.ware', 'serhiy.storchaka', 'yselivanov', 'vajrasky']
    pr_nums = []
    priority = 'critical'
    resolution = 'out of date'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue20075'
    versions = ['Python 3.4']

    @serhiy-storchaka
    Copy link
    Member Author

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

    @serhiy-storchaka serhiy-storchaka added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Dec 26, 2013
    @zware
    Copy link
    Member

    zware commented Dec 26, 2013

    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

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @meadori
    Copy link
    Member

    meadori commented Dec 26, 2013

    Looks like the changes from 78ec18f5cb45 attempt to skip the signature,
    but can't handle multi-line signatures.

    @gennad
    Copy link
    Mannequin

    gennad mannequin commented Dec 27, 2013

    Hi guys,

    probably this patch can fix it?

    @serhiy-storchaka
    Copy link
    Member Author

    LGTM.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Dec 27, 2013

    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

    @larryhastings
    Copy link
    Contributor

    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.

    @serhiy-storchaka
    Copy link
    Member Author

    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.

    @gennad
    Copy link
    Mannequin

    gennad mannequin commented Dec 27, 2013

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

    @zware
    Copy link
    Member

    zware commented Dec 27, 2013

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

    @gennad
    Copy link
    Mannequin

    gennad mannequin commented Dec 27, 2013

    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?

    @gennad
    Copy link
    Mannequin

    gennad mannequin commented Dec 27, 2013

    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.

    @larryhastings
    Copy link
    Contributor

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

    @gennad
    Copy link
    Mannequin

    gennad mannequin commented Dec 27, 2013

    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." ?

    @gennad
    Copy link
    Mannequin

    gennad mannequin commented Jan 1, 2014

    Zachary, thank you for review. Here's the updated patch.

    @zware
    Copy link
    Member

    zware commented Jan 20, 2014

    Gennadiy, sorry this stalled out like it did.

    The patched function will be moving as part of bpo-20189. 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 bpo-20189, please do so.

    (Setting priority as 'critical' since it would be quite embarrassing to release 3.4.0 with broken "help(open)".)

    @skrah
    Copy link
    Mannequin

    skrah mannequin commented Jan 21, 2014

    See also bpo-20326.

    @zware
    Copy link
    Member

    zware commented Jan 28, 2014

    With bpo-20326 fixed, this is no longer an issue.

    Gennadiy, thank you for the patch, and I'm sorry it ended up being unused.

    @zware zware closed this as completed Jan 28, 2014
    @larryhastings
    Copy link
    Contributor

    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!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants