classification
Title: sndhdr.whathdr could return a namedtuple
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: Claudiu.Popa, python-dev, r.david.murray, rhettinger, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-08-01 15:17 by Claudiu.Popa, last changed 2014-10-09 21:00 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
sndhdr.patch Claudiu.Popa, 2013-08-01 15:17 review
issue18615.patch Claudiu.Popa, 2014-07-15 18:04
issue18615_1.patch Claudiu.Popa, 2014-07-15 20:36
issue18615_2.patch Claudiu.Popa, 2014-08-06 16:23 review
Messages (17)
msg194080 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-08-01 15:17
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.
msg203422 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2013-11-19 20:59
Ping, please review. I guess it is minimal enough to get into 3.4.
msg217250 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-04-27 10:17
Ping. :)
msg218436 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-05-13 11:32
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).
msg218439 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-05-13 11:38
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?
msg223091 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-07-15 06:56
Serhiy, if there's no actual gain in changing this, should we close the issue?
msg223092 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-07-15 07:28
This is a reasonable improvement.  It was what named tuples were intended to be used for.
msg223101 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-15 11:24
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?
msg223131 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-07-15 18:04
Thanks, Serhiy, for the review. Here's the updated version.
msg223134 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-07-15 18:30
A namedtuple is still wrongly documented.
msg223147 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-07-15 20:36
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.
msg224948 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-08-06 16:16
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.
msg224951 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-08-06 16:23
Thanks.
msg226211 - (view) Author: Claudiu Popa (Claudiu.Popa) * Date: 2014-09-01 06:42
Serhiy, is there anything left to do for this patch?
msg226218 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2014-09-01 08:48
The patch looks good.  I'll apply it shortly.
msg228909 - (view) Author: Roundup Robot (python-dev) Date: 2014-10-09 21:00
New changeset ef72142eb8a2 by R David Murray in branch 'default':
#18615: Make sndhdr return namedtuples.
https://hg.python.org/cpython/rev/ef72142eb8a2
msg228910 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-10-09 21:00
Committed.  Thanks, Claudiu.
History
Date User Action Args
2014-10-09 21:00:54r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg228910

stage: commit review -> resolved
2014-10-09 21:00:16python-devsetnosy: + python-dev
messages: + msg228909
2014-10-09 18:11:32Claudiu.Popasetstage: patch review -> commit review
2014-09-01 08:48:19rhettingersetmessages: + msg226218
2014-09-01 06:42:40Claudiu.Popasetmessages: + msg226211
2014-08-06 16:23:51Claudiu.Popasetfiles: + issue18615_2.patch

messages: + msg224951
2014-08-06 16:16:49serhiy.storchakasetmessages: + msg224948
2014-07-15 20:36:18Claudiu.Popasetfiles: + issue18615_1.patch

messages: + msg223147
2014-07-15 18:30:42serhiy.storchakasetmessages: + msg223134
2014-07-15 18:04:02Claudiu.Popasetfiles: + issue18615.patch

messages: + msg223131
2014-07-15 11:24:01serhiy.storchakasetmessages: + msg223101
2014-07-15 07:28:57rhettingersetassignee: rhettinger

messages: + msg223092
nosy: + rhettinger
2014-07-15 06:56:40Claudiu.Popasetmessages: + msg223091
2014-05-13 11:38:46Claudiu.Popasetmessages: + msg218439
2014-05-13 11:32:08serhiy.storchakasetmessages: + msg218436
2014-04-27 10:17:52Claudiu.Popasetmessages: + msg217250
2014-03-17 18:36:38Claudiu.Popasetversions: + Python 3.5, - Python 3.4
2013-11-19 20:59:26Claudiu.Popasetmessages: + msg203422
2013-09-04 13:13:14serhiy.storchakasetnosy: + r.david.murray

stage: patch review
2013-09-04 12:25:57Claudiu.Popasetnosy: + serhiy.storchaka
2013-08-01 15:17:55Claudiu.Popacreate