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

sndhdr.whathdr could return a namedtuple #62815

Closed
PCManticore mannequin opened this issue Aug 1, 2013 · 17 comments
Closed

sndhdr.whathdr could return a namedtuple #62815

PCManticore mannequin opened this issue Aug 1, 2013 · 17 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@PCManticore
Copy link
Mannequin

PCManticore mannequin commented Aug 1, 2013

BPO 18615
Nosy @rhettinger, @bitdancer, @PCManticore, @serhiy-storchaka
Files
  • sndhdr.patch
  • issue18615.patch
  • issue18615_1.patch
  • issue18615_2.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/rhettinger'
    closed_at = <Date 2014-10-09.21:00:54.026>
    created_at = <Date 2013-08-01.15:17:55.737>
    labels = ['type-feature', 'library']
    title = 'sndhdr.whathdr could return a namedtuple'
    updated_at = <Date 2014-10-09.21:00:54.025>
    user = 'https://github.com/PCManticore'

    bugs.python.org fields:

    activity = <Date 2014-10-09.21:00:54.025>
    actor = 'r.david.murray'
    assignee = 'rhettinger'
    closed = True
    closed_date = <Date 2014-10-09.21:00:54.026>
    closer = 'r.david.murray'
    components = ['Library (Lib)']
    creation = <Date 2013-08-01.15:17:55.737>
    creator = 'Claudiu.Popa'
    dependencies = []
    files = ['31110', '35956', '35958', '36290']
    hgrepos = []
    issue_num = 18615
    keywords = ['patch']
    message_count = 17.0
    messages = ['194080', '203422', '217250', '218436', '218439', '223091', '223092', '223101', '223131', '223134', '223147', '224948', '224951', '226211', '226218', '228909', '228910']
    nosy_count = 5.0
    nosy_names = ['rhettinger', 'r.david.murray', 'Claudiu.Popa', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue18615'
    versions = ['Python 3.5']

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Aug 1, 2013

    Both sndhdr.whathdr an sndhdr.what returns a tuple with various information, while it could return a namedtuple. I attached a patched for this, with tests as well.

    @PCManticore PCManticore mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Aug 1, 2013
    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Nov 19, 2013

    Ping, please review. I guess it is minimal enough to get into 3.4.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Apr 27, 2014

    Ping. :)

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented May 13, 2014

    Personally I doubt it is a good idea to convert any tuple to named tuple. There are downsides: this increases memory usage and decreases performance; this changes pickled data and makes it backward incompatible (and even worse with other serialization methods).

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented May 13, 2014

    But it improves the API. It's much nicer to actually access the values returned by sndhdr as f.type, f.sampling_rate, f.channels than f[0], f[1], f[2]. You do have a point though. Would it be more acceptable if we'll provide a new function which returns a namedtuple and leaving whathdr and what in peace?

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Jul 15, 2014

    Serhiy, if there's no actual gain in changing this, should we close the issue?

    @rhettinger
    Copy link
    Contributor

    rhettinger commented Jul 15, 2014

    This is a reasonable improvement. It was what named tuples were intended to be used for.

    @rhettinger rhettinger self-assigned this Jul 15, 2014
    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Jul 15, 2014

    If Raymond found this feature helpful, I have no strong objection.

    Only several comments:

    • A named tuple is documented as having fields: type, sampling_rate, channels, frames, bits_per_sample.

    • User tests in existing code return tuples. what() and whathdr() should convert result to named tuple. Changing standard tests after this is redundant.

    • If we guarantee pickleability, we will stick with nametuple's name. It is not more implementation detail which we can change in any moment, all future versions of Python should have sndhdr._SndHeaders. Is it good to use underscored name?

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Jul 15, 2014

    Thanks, Serhiy, for the review. Here's the updated version.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Jul 15, 2014

    A namedtuple is still wrongly documented.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Jul 15, 2014

    Here's a new version. It adds versionchanged directive for 'whathdr' and 'what'. Serhiy, I hope that now I got right the documentation of the return type. I didn't understand at first what was wrong.

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Aug 6, 2014

    The documentation says:

    """
    When these functions are able to determine
    what type of sound data is stored in a file,
    they return a :func:`~collections.namedtuple`, containing five attributes:
    (``type``, ``sampling_rate``, ``channels``,
    ``frames``, ``bits_per_sample``).
    """

    But actual attributes of returned namedtuple are filetype, framerate, nchannels, nframes and sampwidth.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Aug 6, 2014

    Thanks.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Sep 1, 2014

    Serhiy, is there anything left to do for this patch?

    @rhettinger
    Copy link
    Contributor

    rhettinger commented Sep 1, 2014

    The patch looks good. I'll apply it shortly.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Oct 9, 2014

    New changeset ef72142eb8a2 by R David Murray in branch 'default':
    bpo-18615: Make sndhdr return namedtuples.
    https://hg.python.org/cpython/rev/ef72142eb8a2

    @bitdancer
    Copy link
    Member

    bitdancer commented Oct 9, 2014

    Committed. Thanks, Claudiu.

    @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
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants