Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(19)

#17818: aifc.Aifc_read/Aifc_write.getparams can return a namedtuple

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 10 months ago by pcmanticore
Modified:
6 years, 7 months ago
Reviewers:
berker.peksag, rdmurray, storchaka
CC:
r.david.murray, Claudiu.Popa, devnull_psf.upfronthosting.co.za, storchaka
Visibility:
Public.

Patch Set 1 #

Total comments: 1

Patch Set 2 #

Total comments: 3

Patch Set 3 #

Total comments: 4

Patch Set 4 #

Patch Set 5 #

Patch Set 6 #

Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/aifc.rst View 1 2 3 4 5 1 chunk +3 lines, -1 line 0 comments Download
Doc/whatsnew/3.4.rst View 1 2 3 4 5 1 chunk +6 lines, -0 lines 0 comments Download
Lib/aifc.py View 1 2 3 4 5 4 chunks +11 lines, -6 lines 0 comments Download
Lib/test/test_aifc.py View 1 2 3 4 5 3 chunks +32 lines, -0 lines 0 comments Download
Lib/test/test_pyclbr.py View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 3
berkerpeksag
http://bugs.python.org/review/17818/diff/7967/Doc/library/aifc.rst File Doc/library/aifc.rst (right): http://bugs.python.org/review/17818/diff/7967/Doc/library/aifc.rst#newcode99 Doc/library/aifc.rst:99: Return a :func:`~collections.namedtuple` consisting of all of the above ...
6 years, 10 months ago #1
r.david.murray
http://bugs.python.org/review/17818/diff/7973/Lib/test/test_aifc.py File Lib/test/test_aifc.py (right): http://bugs.python.org/review/17818/diff/7973/Lib/test/test_aifc.py#newcode36 Lib/test/test_aifc.py:36: self.assertEqual(f.getnchannels(), params.nchannels) We should still be testing the expected ...
6 years, 10 months ago #2
storchaka_gmail.com
6 years, 10 months ago #3
http://bugs.python.org/review/17818/diff/8005/Doc/library/aifc.rst
File Doc/library/aifc.rst (right):

http://bugs.python.org/review/17818/diff/8005/Doc/library/aifc.rst#newcode99
Doc/library/aifc.rst:99: Return a :func:`~collections.namedtuple` consisting of
all of the above values in the above order.
Perhaps it worth to enumerate these values.

http://bugs.python.org/review/17818/diff/8005/Lib/aifc.py
File Lib/aifc.py (right):

http://bugs.python.org/review/17818/diff/8005/Lib/aifc.py#newcode386
Lib/aifc.py:386: return _result(self.getnchannels(), self.getsampwidth(),
Although it is a private name, it must be more meaningful. I.e. _Aifc_params.

http://bugs.python.org/review/17818/diff/8005/Lib/test/test_aifc.py
File Lib/test/test_aifc.py (right):

http://bugs.python.org/review/17818/diff/8005/Lib/test/test_aifc.py#newcode36
Lib/test/test_aifc.py:36: self.assertEqual(f.getnchannels(), params.nchannels)
As David noted, you should preserve old tests. Just add several lines at the end
of this function which tests attributes of params.

http://bugs.python.org/review/17818/diff/8005/Lib/test/test_aifc.py#newcode52
Lib/test/test_aifc.py:52: self.assertEqual(params.nchannels, 1)
Please add tests for f.getnchannels(), f.getsampwidth(), ... and f.getparams().
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7+