classification
Title: aifc.Aifc_read/Aifc_write.getparams can return a namedtuple
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Claudiu.Popa, python-dev, r.david.murray, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-04-22 19:37 by Claudiu.Popa, last changed 2013-07-25 20:13 by r.david.murray. This issue is now closed.

Files
File name Uploaded Description Edit
aifc.patch Claudiu.Popa, 2013-04-22 19:37 review
aifc_1.patch Claudiu.Popa, 2013-04-23 09:03 add versionchanged tag review
aifc_2.patch Claudiu.Popa, 2013-04-27 04:39 review
aifc_3.patch Claudiu.Popa, 2013-04-28 21:10 review
aifc_4.patch Claudiu.Popa, 2013-07-25 17:57 review
aifc_5.patch Claudiu.Popa, 2013-07-25 19:20 review
Messages (13)
msg187584 - (view) Author: Claudiu.Popa (Claudiu.Popa) * Date: 2013-04-22 19:37
Hello!

Given the fact that issue 17487 was accepted, I think that is a good idea for aifc.Aifc_read/Aifc_write.getparams to return a namedtuple as well, so that both modules will behave consistently with each other. 
I've attached a patch for this. 
Thanks in advance!
msg187885 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-04-27 01:48
I made review comments: patch looks good, tests need a bit of work.  Would you be interested in adding the What's New entry as well? (Doc/whatsnew/3.4).
msg187886 - (view) Author: Claudiu.Popa (Claudiu.Popa) * Date: 2013-04-27 04:39
Hello. I changed the patch according to your review comments. Here is the new version.
msg187920 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-04-27 18:32
There is a regression. The result of getparams() was pickable, but now it is not.

I have added other comments on Rietveld.
msg188024 - (view) Author: Claudiu.Popa (Claudiu.Popa) * Date: 2013-04-28 21:10
I've modified the patch according to your comments, thanks! Now the result of getparams() is picklable again.
msg189834 - (view) Author: Claudiu.Popa (Claudiu.Popa) * Date: 2013-05-22 19:27
Can anyone review the latest patch, please? Thanks.
msg189835 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-05-22 19:51
I've got it on my list, but I'm very busy for the next couple weeks.  If someone else gets to it first that's good too :)
msg193701 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-25 16:43
The patch looks good to me.  Can you add a pickling test?
msg193708 - (view) Author: Claudiu.Popa (Claudiu.Popa) * Date: 2013-07-25 17:57
Here's the new patch. Also, I noticed a test failing when running ./python -m test, pyclbr, complaining about _Aifc_params not present in some dict, but I don't really know how to fix it..
msg193713 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-25 18:57
pyclbr is parsing the source code, and since _Aifc_params is not a class, it does not get detected.  So we just need to add it to the ignore list in the pyclbr test.

I'm also getting this when I run the test with your patch applied:

/home/rdmurray/python/p34/Lib/unittest/case.py:496: ResourceWarning: unclosed file <_io.BufferedReader name='@test_9764_tmp'>
  testMethod()

Looks like you are missing a close or two.

Oh, and it occurs to me that _Aifc_params doesn't follow pep8.  aifc should either be all lower case or, if you are viewing it as a class, it should be _AIFCParams.
msg193714 - (view) Author: Claudiu.Popa (Claudiu.Popa) * Date: 2013-07-25 19:20
Here's the new modifications.
msg193717 - (view) Author: Roundup Robot (python-dev) Date: 2013-07-25 20:12
New changeset 560c6e9d1beb by R David Murray in branch 'default':
#17818: aifc.getparams now returns a namedtuple.
http://hg.python.org/cpython/rev/560c6e9d1beb
msg193718 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2013-07-25 20:13
Thanks, Claudiu.
History
Date User Action Args
2013-07-25 20:13:00r.david.murraysetstatus: open -> closed
resolution: fixed
messages: + msg193718

stage: patch review -> resolved
2013-07-25 20:12:22python-devsetnosy: + python-dev
messages: + msg193717
2013-07-25 19:20:26Claudiu.Popasetfiles: + aifc_5.patch

messages: + msg193714
2013-07-25 18:57:17r.david.murraysetmessages: + msg193713
2013-07-25 17:57:14Claudiu.Popasetfiles: + aifc_4.patch

messages: + msg193708
2013-07-25 16:43:21r.david.murraysetmessages: + msg193701
2013-05-22 19:51:24r.david.murraysetmessages: + msg189835
2013-05-22 19:27:47Claudiu.Popasetmessages: + msg189834
2013-04-28 21:10:19Claudiu.Popasetfiles: + aifc_3.patch

messages: + msg188024
2013-04-27 18:32:37serhiy.storchakasetmessages: + msg187920
2013-04-27 04:39:21Claudiu.Popasetfiles: + aifc_2.patch

messages: + msg187886
2013-04-27 01:48:55r.david.murraysetmessages: + msg187885
stage: patch review
2013-04-24 21:50:10pitrousetnosy: + r.david.murray, serhiy.storchaka
2013-04-23 09:03:21Claudiu.Popasetfiles: + aifc_1.patch
2013-04-22 19:37:34Claudiu.Popacreate