aifc.Aifc_read/Aifc_write.getparams can return a namedtuple

[PCManticore]

BPO	17818
Nosy	@bitdancer, @PCManticore, @serhiy-storchaka
Files	aifc.patch aifc_1.patch: add versionchanged tag aifc_2.patch aifc_3.patch aifc_4.patch aifc_5.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 = None
closed_at = <Date 2013-07-25.20:13:00.443>
created_at = <Date 2013-04-22.19:37:34.414>
labels = ['type-feature', 'library']
title = 'aifc.Aifc_read/Aifc_write.getparams can return a namedtuple'
updated_at = <Date 2013-07-25.20:13:00.441>
user = 'https://github.com/PCManticore'

bugs.python.org fields:

activity = <Date 2013-07-25.20:13:00.441>
actor = 'r.david.murray'
assignee = 'none'
closed = True
closed_date = <Date 2013-07-25.20:13:00.443>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2013-04-22.19:37:34.414>
creator = 'Claudiu.Popa'
dependencies = []
files = ['29980', '29988', '30028', '30052', '31034', '31036']
hgrepos = []
issue_num = 17818
keywords = ['patch']
message_count = 13.0
messages = ['187584', '187885', '187886', '187920', '188024', '189834', '189835', '193701', '193708', '193713', '193714', '193717', '193718']
nosy_count = 4.0
nosy_names = ['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/issue17818'
versions = ['Python 3.4']

[PCManticore]

Hello!

Given the fact that bpo-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!

[bitdancer]

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).

[PCManticore]

Hello. I changed the patch according to your review comments. Here is the new version.

[serhiy-storchaka]

There is a regression. The result of getparams() was pickable, but now it is not.

I have added other comments on Rietveld.

[PCManticore]

I've modified the patch according to your comments, thanks! Now the result of getparams() is picklable again.

[PCManticore]

Can anyone review the latest patch, please? Thanks.

[bitdancer]

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 :)

[bitdancer]

The patch looks good to me. Can you add a pickling test?

[PCManticore]

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..

[bitdancer]

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 PEP-8. aifc should either be all lower case or, if you are viewing it as a class, it should be _AIFCParams.

[PCManticore]

Here's the new modifications.

[python-dev]

New changeset 560c6e9d1beb by R David Murray in branch 'default':
bpo-17818: aifc.getparams now returns a namedtuple.
http://hg.python.org/cpython/rev/560c6e9d1beb

[bitdancer]

Thanks, Claudiu.