http://codereview.appspot.com/602/diff/1/21
File Objects/rangeobject.c (right):

http://codereview.appspot.com/602/diff/1/21#newcode17
Line 17: PyObject *length;
What is the purpose of caching the length in the range
object? The iterators already have a copy of the length.

http://codereview.appspot.com/602/diff/1/22
File Lib/test/test_range.py (right):

http://codereview.appspot.com/602/diff/1/22#newcode72
Line 72: self.assertEqual(r.step, 1)
I'd like the test to be explicit about what happens in some corner cases.  E.g.

r = range(1, 0)  # start=1, stop=0, step=1

r = range(1, 10, 2)  # start=1, stop=10, step=2

http://codereview.appspot.com/602/diff/1/21
File Objects/rangeobject.c (right):

http://codereview.appspot.com/602/diff/1/21#newcode17
Line 17: PyObject *length;
On 2008/05/02 05:43:57, mvloewis wrote:
> What is the purpose of caching the length in the range
> object? The iterators already have a copy of the length.

See line 651 -- the range_reverse() function uses it.

http://codereview.appspot.com/602/diff/1/21#newcode48
Line 48: PyObject *stop, PyObject *step);
This line is indented one space too many.

http://codereview.appspot.com/602/diff/1/21#newcode111
Line 111: Returns a virtual sequence of numbers from start to stop by step.");
I really don't want to use the word sequence here.  Maybe:

Returns an arithmetic progression of numbers from start to stop (exclusive) by
step.

http://codereview.appspot.com/602/diff/1/21#newcode127
Line 127: * PyLong_Check().  Return -1 when there is an error.
Not your fault, but this comment still references PyInt vs. PyLong, *and*
"either PyLong_Check() or PyLong_Check()" makes no sense (must've been
automatically changed in the past from "PyInt_Check() or PyLong_Check()").  Mind
fixing this?

http://codereview.appspot.com/602/diff/1/21#newcode254
Line 254: (lenfunc)range_length,	/* sq_length */
The latest discussion on the list suggests to remove __len__ and instead make
this value available as __length_hint__ (or whatever the magic method is).

http://codereview.appspot.com/602/diff/1/21#newcode306
Line 306: range_getset,		/* tp_getset */
Please use spaces to indent the comment.

Hey musiccomposition, can you add the second patch from the bug issue to this
review issue?

http://codereview.appspot.com/602/diff/1/21
File Objects/rangeobject.c (right):

http://codereview.appspot.com/602/diff/1/21#newcode17
Line 17: PyObject *length;
On 2008/05/02 05:43:57, mvloewis wrote:
> What is the purpose of caching the length in the range
> object? The iterators already have a copy of the length.

In the next patch, I'm also going to use it in range_iter.

http://codereview.appspot.com/602/diff/141/83
File Objects/rangeobject.c (right):

http://codereview.appspot.com/602/diff/141/83#newcode288
Line 288: range_members,		/* tp_members */
Carrying forward from the previous patch set: this should use spaces.

http://codereview.appspot.com/602/diff/141/83#newcode634
Line 634: len = range->length;
You need to INCREF, since you are a) going to DECREF in case of failure, and b)
stealing the reference later.

Here are a few more comments, Benjamin.

http://codereview.appspot.com/602/diff/243/183
File Lib/test/test_range.py (right):

http://codereview.appspot.com/602/diff/243/183#newcode71
Line 71: self.assertEqual(r.step, 4)
Well how about

r = range(1, 10, 2) # start=1, stop=10, step=2

?  (Carried over from previous review.)

http://codereview.appspot.com/602/diff/243/182
File Objects/rangeobject.c (right):

http://codereview.appspot.com/602/diff/243/182#newcode124
Line 124: 
Hm. I didn't mean for you to delete the entire comment, just to clean it up. :-)

http://codereview.appspot.com/602/diff/222/302
File Lib/ctypes/test/test_slicing.py (right):

http://codereview.appspot.com/602/diff/222/302#newcode23
Line 23: a[0:5] = list(range(5, 10))
Hm... Why can't you assign a range() to a c_int array? Is it because it must be
a list, or because it must be a sequence?  (I'm guessing ctypes ought to accept
any iterable...)

http://codereview.appspot.com/602/diff/222/306
File Lib/test/pickletester.py (right):

http://codereview.appspot.com/602/diff/222/306#newcode588
Line 588: oob = list(protocols)[-1] + 1     # a future protocol
The list() call should be moved to line 13, where the variable `protocols` is
set.

one reply

http://codereview.appspot.com/602/diff/222/302
File Lib/ctypes/test/test_slicing.py (right):

http://codereview.appspot.com/602/diff/222/302#newcode23
Line 23: a[0:5] = list(range(5, 10))
On 2008/05/03 05:04:35, gvanrossum wrote:
> Hm... Why can't you assign a range() to a c_int array? Is it because it must
be
> a list, or because it must be a sequence?  (I'm guessing ctypes ought to
accept
> any iterable...)

It's because PySequence_GetItem is called on the range. You'd have to ask Thomas
any other questions, though.

Feel free to close this, the corresponding bug has been closed (rejected).