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

#23704: Make deques a full MutableSequence by adding index(), insert(), and copy()

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 3 months ago by raymond.hettinger
Modified:
4 years, 2 months ago
Reviewers:
storchaka
CC:
rhettinger, devnull_psf.upfronthosting.co.za, storchaka
Visibility:
Public.

Patch Set 1 #

Patch Set 2 #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/collections.rst View 1 2 chunks +22 lines, -0 lines 0 comments Download
Lib/collections/__init__.py View 1 1 chunk +2 lines, -0 lines 0 comments Download
Lib/test/test_deque.py View 1 2 chunks +57 lines, -0 lines 5 comments Download
Modules/_collectionsmodule.c View 1 2 chunks +91 lines, -0 lines 3 comments Download

Messages

Total messages: 1
storchaka_gmail.com
4 years, 2 months ago #1
http://bugs.python.org/review/23704/diff/14294/Lib/test/test_deque.py
File Lib/test/test_deque.py (right):

http://bugs.python.org/review/23704/diff/14294/Lib/test/test_deque.py#newcode247
Lib/test/test_deque.py:247: with self.assertRaises(RuntimeError):
May be add a test that d.index(n//2-1) doesn't fail (if n > 1)?

http://bugs.python.org/review/23704/diff/14294/Lib/test/test_deque.py#newcode261
Lib/test/test_deque.py:261: for start in range(-5 - len(s)*2, 5 + len(s) * 2):
I think "* 2" is not needed in the upper limit.

http://bugs.python.org/review/23704/diff/14294/Lib/test/test_deque.py#newcode263
Lib/test/test_deque.py:263: for element in elements + 'Z':
May be subTest() would be useful here?

http://bugs.python.org/review/23704/diff/14294/Lib/test/test_deque.py#newcode275
Lib/test/test_deque.py:275: for i in range(-5 - len(elements)*2, 5 +
len(elements) * 2):
I think "* 2" is not needed in the upper limit.

http://bugs.python.org/review/23704/diff/14294/Lib/test/test_deque.py#newcode572
Lib/test/test_deque.py:572: self.assertNotEqual(id(d), id(e))
Why not assertIsNot()?

http://bugs.python.org/review/23704/diff/14294/Modules/_collectionsmodule.c
File Modules/_collectionsmodule.c (right):

http://bugs.python.org/review/23704/diff/14294/Modules/_collectionsmodule.c#n...
Modules/_collectionsmodule.c:789: for (i=0 ; i<stop ; i++) {
Strange spacing.

http://bugs.python.org/review/23704/diff/14294/Modules/_collectionsmodule.c#n...
Modules/_collectionsmodule.c:805: index++;
The iteration to start can be faster. I would split the loop on two loops. First
from 0 to start, with large steps of size BLOCKLEN, and then from start to stop,
without testing i >= start. And may be the second loop can be shared with
deque_contains (not sure).

http://bugs.python.org/review/23704/diff/14294/Modules/_collectionsmodule.c#n...
Modules/_collectionsmodule.c:839: if (rv == NULL)
The deque is left changed.
Sign in to reply to this message.

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