Repository navigation
Slice with negative step inconsistent with python result #312
Description
First of all, I understand that this crate is not meant to reproduce all feature in python, but since in the docs about Si python slice is used as comparison, so I believe this is worth to bring up.
Here is some test case with ndarray:
#[macro_use(s)]
extern crate ndarray;
use ndarray::prelude::*;
fn main() {
let x: Array1<usize> = Array::zeros(16);
println!("{}", x.slice(s![..4;1]).len());
println!("{}", x.slice(s![4..;1]).len());
println!("{}", x.slice(s![..-4;1]).len());
println!("{}", x.slice(s![-4..;1]).len());
println!("{}", x.slice(s![..4;-1]).len());
println!("{}", x.slice(s![4..;-1]).len());
println!("{}", x.slice(s![..-4;-1]).len());
println!("{}", x.slice(s![-4..;-1]).len());
}
which return:
4
12
12
4
4
12
12
4
and in the corresponding python test case (in both python 2.7.6 and python 3.4.3):
def main():
x = [0] * 16
print(len(x[:4:1]))
print(len(x[4::1]))
print(len(x[:-4:1]))
print(len(x[-4::1]))
print(len(x[:4:-1]))
print(len(x[4::-1]))
print(len(x[:-4:-1]))
print(len(x[-4::-1]))
if __name__ == '__main__':
main()
which return:
4
12
12
4
11
5
3
13
It seems that, with negative step, the default begin is n - 1 and the default end is 0 for python. To be honest, I prefer the ndarray approach to kept the default consistance, but I believe this should either mention in the docs or changed.
Activity
bluss commentedon Apr 20, 2017
Interesting. This was definitely a surprise to me:
jturner314 commentedon Nov 24, 2017
I just noticed this. In #377, I copied the existing behavior.
Other than the Python and
ndarray 0.10approaches, there's a third option I think is the best fit forndarray. These are examples of the three options (indexing an axis of length4(indices0,1,2,3)):2..0;-1→2,1..0;-2→3,13..0;-2→3,13..;-2→3,1..;-2→3,1ndarray 0.101..3;-1→2,11..;-2→3,10..4;-2→3,1..4;-2→3,1..;-2→3,11..3;-1→2,11..;-2→3,10..4;-2→2,0..4;-2→2,0..;-2→2,0Option 1 makes sense to me since I've used Python for a while, but it doesn't match the documentation for
std::ops::Range(which specifies thatstart..endcontains valuesxwithx >= startandx < end).Option 2 matches the docs for
std::ops::Rangebut behaves in surprising ways forstep < -1. Examples i and ii make sense, but examples iii and iv are surprising because none of the indices are equal tostartorend.I think option 3 is the best fit for
ndarray. It matches the docs forstd::ops::Range, and the last index always equalsstart. With this approach,start..end;-stepis the same asreverse(start..end;step).What do you think? If you'd like to change the behavior, the 0.11 release would be a good time to do so. Either way, I can add more documentation to explain how negative steps are handled.
bluss commentedon Nov 24, 2017
Great to see the details!
My default would be to not change the behaviour, so it would be best to have both the logical reason and a technical reason (like: fits this algorithm better or can be implemented with less conversion / checking loss etc).
As a rebuttal, the last element in
0..4is a3, and that's the reason that is the first element when the step is negative. It does seem to indicate a difference in perspective. Ndarray seems to think that we first slice, then reverse, then count steps.Python just seems to do one thing differently -- it swaps the ends of
i..jto bej..iwhen the step is negative, and it also swaps which of them is inclusive? I suppose swapping inclusiveness is not how it should be understood, but that's what it looks like compared to Rust ranges. (And in Rust we have a precedent for not doing that -- we use(i..j).rev()but notj..iwherei <= j.)jturner314 commentedon Nov 25, 2017
I don't have a strong preference, although I think options 2 and 3 fit Rust conventions a little better than option 1.
Python's approach (option 1) to
start:end:stepis "the first index isstart, then addstepuntil reachingend(exclusive)".ndarray 0.10's approach (option 2) can be expressed in a few different (equivalent) ways:stepis positive, then the first index isstart, and then addstepuntil reachingend(exclusive). Ifstepis negative, then the first index isend - 1, and then addstepuntil reachingstart(inclusive)."start..end. Ifstepis negative, reverse the slice. Start at the front of the (possibly reversed) slice, and addstep.abs()until reaching the back of the slice (inclusive)."start..end. Ifstepis positive, start with the front of the slice; ifstepis negative, start with the back of the slice. Then, addstepuntil reaching the other end of the slice (inclusive)."Option 3's approach is "If
stepis positive, then the first index isstart, and then addstepuntil reachingend(exclusive). Ifstepis negative, then the indices are the same as ifstepwere positive; just reverse the order."Note that if we keep
ndaray 0.10's behavior (option 2), it would be easy to add as_py![a;b;c, d;e;f, ...]macro which interprets its arguments like Python and returns aSliceInfoinstance. This might be useful for users who want to translate NumPy algorithms tondarray. I don't think it's possible to do this if we change the default behavior to option 3, because the lengths of the axes would need to be known.Keeping the existing behavior fine with me. I don't see any technical reason to prefer one approach over another (other than the ability to add
s_py!if we keepndarray 0.10's behavior). It's also worth noting that if we change the behavior, it might be difficult for users to update their code if they're doing complex logic with slice indices.