When calling the indices method of a slice object with a negative stop
larger in absolute value than the length passed, the returned stop value
is always -1. It should be 0 when the step is positive.

Current behavior:

>>> slice(-10).indices(11)
(0, 1, 1)
>>> slice(-10).indices(10)
(0, 0, 1)
>>> slice(-10).indices(9)
(0, -1, 1)
>>> slice(-10).indices(8)
(0, -1, 1)

Expected behavior:

>>> slice(-10).indices(11)
(0, 1, 1)
>>> slice(-10).indices(10)
(0, 0, 1)
>>> slice(-10).indices(9)
(0, 0, 1)
>>> slice(-10).indices(8)
(0, 0, 1)

The patch I submit trivially fixes this while preserving the expected -1
when the step is negative like in:

>>> slice(None, -10, -1).indices(8)
(7, -1, -1)

This issue affects python 2.5 and 2.6 at least. I did not test with
other versions.

I agree that those -1s should really be 0s.

Do you have any examples of real-life code
that's affected by this bug? It doesn't seem
like something that would be a problem in
practice.

It's for code that I am developping. I developped a class to allow
full slicing over iterators (like what islice does, but with negative
indexes). When I have a positive step I just foward the call to
isclice using slice.indices() to compute my limits. But with this
behavior, I am forced to wrap the indices() call with a function that
patches this case.

I agree that for the common usage of computing the limits of a for
loop it doesn't matter. But it is really surprising when you realise
this behavior is what is causing these exceptions after having passed
half a day trying to debug your code.

In fact, I think this bug should really be more of a documentation bug
and I should propose a patch to add clearer documentation to this
method. But the fix proposed should also go in because:

• it's trivial
• it's better

Documentation for the method will be submitted later tomorrow. Should
I post another report or just attach it to this one?

Don't blame me for the delay, I have long days (yes, really up to 96
hours long :)

As for the documentation patch, I'm not certain anymore about it.
Unless I bloat the description to about one full screen worth of text,
there will always be surprises for the user. And describing only one
little part in detail feels inconsistent.

So unless I'm just a really bad writer, I think the current doc could be
left as-is. I'm still all for the attached patch. Comments anyone? Or
does this needs more nagging?

Could you provide some tests for the fixed behaviour?

I'll try to check this in (with appropriate tests) after the beta.

Would these do?

self.assertEqual(slice(None, -10 ).indices(10), (0, 0, 1))
self.assertEqual(slice(None, -11, ).indices(10), (0, 0, 1))
self.assertEqual(slice(None, -12, -1).indices(10), (9, -1, -1))

If yes, test_slice.patch adds them.

On Wed, Jun 18, 2008 at 4:56 PM, Arnaud Bergeron <report@bugs.python.org>
wrote:

Arnaud Bergeron <abergeron@gmail.com> added the comment:

Would these do?

self.assertEqual(slice(None, -10 ).indices(10), (0, 0, 1))
self.assertEqual(slice(None, -11, ).indices(10), (0, 0, 1))
self.assertEqual(slice(None, -12, -1).indices(10), (9, -1, -1))

Perfect. Thank you.

If this is changed, then I think the following should also be
changed:

(9, 10, -1)

I believe the second index here should be 9, not 10.
Do you agree?

With these two changes the code, while marginally
more complicated, is actually easier to understand than
before, since exactly the same processing is applied
to both the start and stop indices. I think this is as
it should be.

Sorry: looks like I messed up that last post. The example should be:

>>> slice(10, 10, -1).indices(10)  # expect (9, 9, -1)
(9, 10, -1)

Here's a new patch that incorporates Arnaud's fix and tests, together with
a few extra tests.

While I expect that this change will affect very little code, I think it's
the right thing to do, because:

• start and stop are now processed identically, making the source code
  easier to understand, and the behaviour easier to explain.

• expected invariants now hold even in corner cases; for example, after:

start, stop, step = my_slice.indices(length)

it's guaranteed that

0 <= start <= stop <= length if step is positive, and
length-1 >= start >= stop >= -1 if step is negative.

However, I'd like a second opinion from another core developer before
checking this in.

0 <= start <= stop <= length if step is positive, and
length-1 >= start >= stop >= -1 if step is negative.

That should be:

0 <= start <= length and 0 <= stop <= length (step > 0), and
length-1 >= start >= -1, length-1 >= stop >= -1 (step < 0);

it's not guaranteed that start <= stop always holds in the first case,
or that start >= stop in the second.

Looks like a straight-forward patch.

Committed, r64426.

Thanks for the report, Arnaud.