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

Inconsistent docstrings in struct module #53219

Closed
abalkin opened this issue Jun 11, 2010 · 19 comments
Closed

Inconsistent docstrings in struct module #53219

abalkin opened this issue Jun 11, 2010 · 19 comments
Assignees
Labels
extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error

Comments

@abalkin
Copy link
Member

abalkin commented Jun 11, 2010

BPO 8973
Nosy @birkenfeld, @mdickinson, @abalkin
Files
  • issue8973.patch
  • issue8973-Struct.diff: Expanded Struct.doc
  • struct.py.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/abalkin'
    closed_at = <Date 2011-01-31.17:23:54.870>
    created_at = <Date 2010-06-11.15:32:36.870>
    labels = ['extension-modules', 'type-bug']
    title = 'Inconsistent docstrings in struct module'
    updated_at = <Date 2011-01-31.17:23:54.869>
    user = 'https://github.com/abalkin'

    bugs.python.org fields:

    activity = <Date 2011-01-31.17:23:54.869>
    actor = 'SilentGhost'
    assignee = 'belopolsky'
    closed = True
    closed_date = <Date 2011-01-31.17:23:54.870>
    closer = 'SilentGhost'
    components = ['Extension Modules']
    creation = <Date 2010-06-11.15:32:36.870>
    creator = 'belopolsky'
    dependencies = []
    files = ['17643', '17649', '20625']
    hgrepos = []
    issue_num = 8973
    keywords = ['patch']
    message_count = 19.0
    messages = ['107556', '107557', '107558', '107655', '107656', '107657', '107658', '107659', '107660', '107663', '107664', '107666', '107686', '107687', '107688', '127593', '127606', '127609', '127626']
    nosy_count = 4.0
    nosy_names = ['georg.brandl', 'mark.dickinson', 'belopolsky', 'SilentGhost']
    pr_nums = []
    priority = 'normal'
    resolution = 'accepted'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue8973'
    versions = ['Python 3.2']

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 11, 2010

    Module level pack, unpack etc. methods have similar functionality with Struct instance methods, but docs are different. The immediate issue is the lack of signature in the module level methods' docstrings.

    $ ./python.exe -m pydoc struct.Struct.pack
    Help on method_descriptor in struct.Struct:
    struct.Struct.pack = pack(...)
        S.pack(v1, v2, ...) -> bytes
        
        Return a bytes containing values v1, v2, ... packed according to this
        Struct's format. See struct.__doc__ for more on format strings.

    and

    $ ./python.exe -m pydoc struct.pack
    Help on built-in function pack in struct:
    struct.pack = pack(...)
        Return bytes containing values v1, v2, ... packed according to fmt.

    @abalkin abalkin added extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error labels Jun 11, 2010
    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 11, 2010

    Two more bits:

    1. "See struct.__doc__", while technically correct, is not user friendly.  If you copy struct.__doc__ to >>> prompt, you get an ugly repr of a multiline string.  I suggest s/struct.__doc__/help(struct)/.
    1. For some reason struct.Struct does not show up in help(struct).

    @mdickinson
    Copy link
    Member

    Thanks for the reports; I'll look at this a little later.

    @mdickinson mdickinson self-assigned this Jun 11, 2010
    @mdickinson
    Copy link
    Member

    Here's a patch. Alexander, do you want to check it for sanity?

    @mdickinson
    Copy link
    Member

    N.B. The patch doesn't fix the missing struct.Struct documentation issue; I'll look at that separately.

    @mdickinson
    Copy link
    Member

    The docstrings need to be rewrapped to 72 characters; I'll do this once the wording is settled.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 12, 2010

    calcsize() still does not have signature in docstring.

    A nit: maybe follow calcsize() lead and say "format string fmt" rather than just "fmt".

    One more __doc__ mention: in Struct docstring,

    | __init__(...)
    | x.__init__(...) initializes x; see x.__class__.__doc__ for signature

    I think this is inherited, but quite confusing here because help(Struct) does not have signature. Maybe just add signature to help(Struct).

    @mdickinson
    Copy link
    Member

    r81947 fixes the missing struct.Struct entry in struct.help. (pydoc was relying on the inspect module to correctly guess the module for struct.Struct, and inspect was getting it wrong. Adding __all__ to the struct module saves inspect from having to guess.)

    @mdickinson
    Copy link
    Member

    I also accidentally included the docstring changes in r81947; those were reverted in r81948.

    @mdickinson
    Copy link
    Member

    Committed patch (with additional changes suggested by Alexander) in r81949. Only the __init__ doc issue remains.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 12, 2010

    See issue bpo-8983 for the the __init__ doc discussion since it is not specific to the struct module.

    @mdickinson
    Copy link
    Member

    Apart from the pydoc __init__ issue, the Struct class documentation should probably be expanded a bit. At the moment, it's just:

    PyDoc_STRVAR(s__doc__, "Compiled struct object");

    Alexander, interested in producing a patch?

    @mdickinson mdickinson assigned abalkin and unassigned mdickinson Jun 12, 2010
    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 12, 2010

    How does bpo-8973-Struct.diff look?

    @mdickinson
    Copy link
    Member

    Looks fine. I'd probably call the argument 'fmt' rather than 'format', for consistency. Please commit, with or without the 'format' -> 'fmt' change, as you choose.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jun 12, 2010

    Committed in r81961. Yes, I used "format" for consistency with the manual, but on the second thought consistency within help() is more important.

    @abalkin abalkin closed this as completed Jun 12, 2010
    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Jan 31, 2011

    r81947 introduced this issue:

    >>> from struct import *
    >>> pack_into
    Traceback (most recent call last):
      File "<pyshell#1>", line 1, in <module>
        pack_into
    NameError: name 'pack_into' is not defined

    struct.__all__ has a duplicate entry and misses pack_into.

    Patch is attached, test passes. Seems quite innocent to me, could it make into 3.2, George?

    @SilentGhost SilentGhost mannequin reopened this Jan 31, 2011
    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 31, 2011

    The proposed patch looks fine to me, but it is attached to the wrong issue. It belongs to bpo-8973 or better yet to follow RC2 rules pedantically, it should be posted in a separate issue.

    This is important, because this issue is limited to docstrings and may be considered documentation only, but the proposed parch affects behavior. Still I would be +1 on applying it.

    @abalkin
    Copy link
    Member Author

    abalkin commented Jan 31, 2011

    Wait, bpo-8973 *is* this issue. But r81947 is clearly not only about docstrings. We definitely need a separate issue. This is too confusing.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Jan 31, 2011

    new issue bpo-11081 was created for struct.__all__ fix

    @SilentGhost SilentGhost mannequin closed this as completed Jan 31, 2011
    @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
    extension-modules C modules in the Modules dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants