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

Jump to bottom

wave.Wave_read.getparams should be more user friendly #61689

Closed
PCManticore mannequin opened this issue Mar 19, 2013 · 16 comments
Closed

wave.Wave_read.getparams should be more user friendly #61689

PCManticore mannequin opened this issue Mar 19, 2013 · 16 comments
Assignees
@serhiy-storchaka
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@PCManticore
Copy link
Mannequin

PCManticore mannequin commented Mar 19, 2013

BPO 17487
Nosy @benjaminp, @bitdancer, @PCManticore, @berkerpeksag, @serhiy-storchaka
Files
  • wave.patch
  • wave_17487.patch
  • wave_with_docs.patch
  • wave_picklable.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/serhiy-storchaka'
closed_at = <Date 2013-09-03.22:06:15.152>
created_at = <Date 2013-03-19.20:59:09.764>
labels = ['type-feature', 'library']
title = 'wave.Wave_read.getparams should be more user friendly'
updated_at = <Date 2013-09-03.22:06:15.151>
user = 'https://github.com/PCManticore'

    bugs.python.org fields:

    activity = <Date 2013-09-03.22:06:15.151>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2013-09-03.22:06:15.152>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2013-03-19.20:59:09.764>
creator = 'Claudiu.Popa'
dependencies = []
files = ['29488', '29499', '29542', '30245']
hgrepos = []
issue_num = 17487
keywords = ['patch']
message_count = 16.0
messages = ['184680', '184683', '184744', '184899', '184940', '184942', '184947', '184968', '184969', '184988', '186385', '186514', '186515', '189110', '196870', '196873']
nosy_count = 6.0
nosy_names = ['benjamin.peterson', 'r.david.murray', 'Claudiu.Popa', 'python-dev', 'berker.peksag', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17487'
versions = ['Python 3.4']
    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Mar 19, 2013

    wave.Wave_read/Wave_write.getparams returns a tuple with various info about the wav file, when it could return a namedtuple, that can be manipulated in a richer way. I attached a patch. It doesn't have any tests, mainly because .getparams isn't tested at all in the original test_wave.py.

    @PCManticore PCManticore mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Mar 19, 2013
    @bitdancer
    Copy link
    Member

    This seems like a reasonable idea to me, thanks for the patch.

    Adding tests for getparams would be a good thing ;)

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Mar 20, 2013

    Updated the patch with test for .getparams.

    @bitdancer
    Copy link
    Member

    Looks good.

    Claudio, could you please submit a contributor agreement? (http:://www.python.org/psf/contrib).

    @berkerpeksag
    Copy link
    Member

    Should the documentation of the wave module be updated to match this change? See bpo-16808 for example.

    The current wave.Wave_read.getparams documentation says:

    "Returns a tuple (nchannels, sampwidth, framerate, nframes, comptype, compname), equivalent to output of the get*() methods."

    http://docs.python.org/3.4/library/wave.html#wave.Wave_read.getparams

    @bitdancer
    Copy link
    Member

    It should. Here's the patch I am prepared to commit once Claudio's contributor for confirmation occurs.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Mar 22, 2013

    Hello, I signed the electronic contributor agreement (I received a mail with the final copy afterwards). Is there something else that I should do or that is the entire process?

    @benjaminp
    Copy link
    Contributor

    I backed it out until it can be done without breaking OSX. d174cb3f5b9e

    @benjaminp
    Copy link
    Contributor

    Wrong issue, sorry.

    @bitdancer
    Copy link
    Member

    Claudiu: that's the entire process. Now we just wait a day or two until the confirmation arrives from the PSF that they have accepted it (you'll get an '*' next to your name here in the tracker when that happens).

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented Apr 9, 2013

    Hello. I received the confirmation for my CLA, could you commit the patch? Thank you!

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Apr 10, 2013

    New changeset 340a12c18b7f by R David Murray in branch 'default':
    bpo-17487: wave.getparams now returns a namedtuple.
    http://hg.python.org/cpython/rev/340a12c18b7f

    @bitdancer
    Copy link
    Member

    Thanks, Claudiu.

    @PCManticore
    Copy link
    Mannequin Author

    PCManticore mannequin commented May 13, 2013

    There is a regression with the latest patch, getparams output is no longer picklable. The attached patch fixes this issue. Also, I renamed the internal namedtuple to something more meaningful.

    @PCManticore PCManticore mannequin reopened this May 13, 2013
    @serhiy-storchaka serhiy-storchaka self-assigned this Sep 3, 2013
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Sep 3, 2013

    New changeset a14ec46de0a4 by Serhiy Storchaka in branch 'default':
    Issue bpo-17487: The result of the wave getparams method now is pickleable again.
    http://hg.python.org/cpython/rev/a14ec46de0a4

    @serhiy-storchaka
    Copy link
    Member

    Thank you Claudiu. I have changed _Wave_params to _wave_params for consistency with bpo-17818 and bpo-18901.

    @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
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    4 participants
    @berkerpeksag @benjaminp @bitdancer @serhiy-storchaka