Make slice_collapse panic on NewAxis

Author: jturner314

src/impl_methods.rs

@@ -461,12 +461,15 @@ where
 
     /// Slice the array in place without changing the number of dimensions.
     ///
-    /// Note that `NewAxis` elements in `info` are ignored.
-    ///
     /// See [*Slicing*](#slicing) for full documentation.
     ///
-    /// **Panics** if an index is out of bounds or step size is zero.<br>
-    /// (**Panics** if `D` is `IxDyn` and `info` does not match the number of array axes.)
+    /// **Panics** in the following cases:
+    ///
+    /// - if an index is out of bounds
+    /// - if a step size is zero
+    /// - if [`AxisSliceInfo::NewAxis`] is in `info`, e.g. if [`NewAxis`] was
+    ///   used in the [`s!`] macro
+    /// - if `D` is `IxDyn` and `info` does not match the number of array axes
     pub fn slice_collapse<I>(&mut self, info: &I)
     where
         I: CanSlice<D> + ?Sized,
@@ -487,7 +490,7 @@ where
                     self.collapse_axis(Axis(axis), i_usize);
                     axis += 1;
                 }
-                AxisSliceInfo::NewAxis => {}
+                AxisSliceInfo::NewAxis => panic!("`slice_collapse` does not support `NewAxis`."),
             });
         debug_assert_eq!(axis, self.ndim());
     }

src/lib.rs

@@ -498,7 +498,7 @@ pub type Ixs = isize;
 /// is selected and the axis is removed; this selects a subview. See
 /// [*Subviews*](#subviews) for more information about subviews. If a
 /// [`NewAxis`] instance is used, a new axis is inserted. Note that
-/// [`.slice_collapse()`] ignores `NewAxis` elements and behaves like
+/// [`.slice_collapse()`] panics on `NewAxis` elements and behaves like
 /// [`.collapse_axis()`] by preserving the number of dimensions.
 ///
 /// [`.slice()`]: #method.slice

src/slice.rs

@@ -723,6 +723,7 @@ impl_slicearg!((), NewAxis, Ix0, Ix1);
 /// * *slice* `;` *step*: a range constructed from the start and end of a [`Slice`]
 ///   instance, with new step size *step*, to use for slicing that axis.
 /// * *new-axis*: a [`NewAxis`] instance that represents the creation of a new axis.
+///   (Except for [`.slice_collapse()`], which panics on [`NewAxis`] elements.)
 ///
 /// [`Slice`]: struct.Slice.html
 /// [`NewAxis`]: struct.NewAxis.html
@@ -734,13 +735,14 @@ impl_slicearg!((), NewAxis, Ix0, Ix1);
 /// `RangeFull` where `I` is `isize`, `usize`, or `i32`. *step* must be a type
 /// that can be converted to `isize` with the `as` keyword.
 ///
-/// For example `s![0..4;2, 6, 1..5, NewAxis]` is a slice of the first axis for
-/// 0..4 with step size 2, a subview of the second axis at index 6, a slice of
-/// the third axis for 1..5 with default step size 1, and a new axis of length
-/// 1 at the end of the shape. The input array must have 3 dimensions. The
-/// resulting slice would have shape `[2, 4, 1]` for [`.slice()`],
-/// [`.slice_mut()`], and [`.slice_move()`], and shape `[2, 1, 4]` for
-/// [`.slice_collapse()`].
+/// For example, `s![0..4;2, 6, 1..5, NewAxis]` is a slice of the first axis
+/// for 0..4 with step size 2, a subview of the second axis at index 6, a slice
+/// of the third axis for 1..5 with default step size 1, and a new axis of
+/// length 1 at the end of the shape. The input array must have 3 dimensions.
+/// The resulting slice would have shape `[2, 4, 1]` for [`.slice()`],
+/// [`.slice_mut()`], and [`.slice_move()`], while [`.slice_collapse()`] would
+/// panic. Without the `NewAxis`, i.e. `s![0..4;2, 6, 1..5]`,
+/// [`.slice_collapse()`] would result in an array of shape `[2, 1, 4]`.
 ///
 /// [`.slice()`]: struct.ArrayBase.html#method.slice
 /// [`.slice_mut()`]: struct.ArrayBase.html#method.slice_mut

tests/array.rs

@@ -201,7 +201,8 @@ fn test_slice_array_fixed() {
     arr.slice(info);
     arr.slice_mut(info);
     arr.view().slice_move(info);
-    arr.view().slice_collapse(info);
+    let info2 = s![1.., 1, ..;2];
+    arr.view().slice_collapse(info2);
 }
 
 #[test]
@@ -211,7 +212,8 @@ fn test_slice_dyninput_array_fixed() {
     arr.slice(info);
     arr.slice_mut(info);
     arr.view().slice_move(info);
-    arr.view().slice_collapse(info);
+    let info2 = s![1.., 1, ..;2];
+    arr.view().slice_collapse(info2);
 }
 
 #[test]
@@ -227,7 +229,13 @@ fn test_slice_array_dyn() {
     arr.slice(info);
     arr.slice_mut(info);
     arr.view().slice_move(info);
-    arr.view().slice_collapse(info);
+    let info2 = &SliceInfo::<_, Ix3, IxDyn>::try_from([
+        AxisSliceInfo::from(1..),
+        AxisSliceInfo::from(1),
+        AxisSliceInfo::from(..).step_by(2),
+    ])
+    .unwrap();
+    arr.view().slice_collapse(info2);
 }
 
 #[test]
@@ -243,7 +251,13 @@ fn test_slice_dyninput_array_dyn() {
     arr.slice(info);
     arr.slice_mut(info);
     arr.view().slice_move(info);
-    arr.view().slice_collapse(info);
+    let info2 = &SliceInfo::<_, Ix3, IxDyn>::try_from([
+        AxisSliceInfo::from(1..),
+        AxisSliceInfo::from(1),
+        AxisSliceInfo::from(..).step_by(2),
+    ])
+    .unwrap();
+    arr.view().slice_collapse(info2);
 }
 
 #[test]
@@ -259,7 +273,13 @@ fn test_slice_dyninput_vec_fixed() {
     arr.slice(info);
     arr.slice_mut(info);
     arr.view().slice_move(info);
-    arr.view().slice_collapse(info);
+    let info2 = &SliceInfo::<_, Ix3, Ix2>::try_from(vec![
+        AxisSliceInfo::from(1..),
+        AxisSliceInfo::from(1),
+        AxisSliceInfo::from(..).step_by(2),
+    ])
+    .unwrap();
+    arr.view().slice_collapse(info2);
 }
 
 #[test]
@@ -275,7 +295,13 @@ fn test_slice_dyninput_vec_dyn() {
     arr.slice(info);
     arr.slice_mut(info);
     arr.view().slice_move(info);
-    arr.view().slice_collapse(info);
+    let info2 = &SliceInfo::<_, Ix3, IxDyn>::try_from(vec![
+        AxisSliceInfo::from(1..),
+        AxisSliceInfo::from(1),
+        AxisSliceInfo::from(..).step_by(2),
+    ])
+    .unwrap();
+    arr.view().slice_collapse(info2);
 }
 
 #[test]
@@ -324,35 +350,42 @@ fn test_slice_collapse_with_indices() {
 
     {
         let mut vi = arr.view();
-        vi.slice_collapse(s![NewAxis, 1.., 2, ..;2]);
+        vi.slice_collapse(s![1.., 2, ..;2]);
         assert_eq!(vi.shape(), &[2, 1, 2]);
         assert!(vi
             .iter()
             .zip(arr.slice(s![1.., 2..3, ..;2]).iter())
             .all(|(a, b)| a == b));
 
         let mut vi = arr.view();
-        vi.slice_collapse(s![1, NewAxis, 2, ..;2]);
+        vi.slice_collapse(s![1, 2, ..;2]);
         assert_eq!(vi.shape(), &[1, 1, 2]);
         assert!(vi
             .iter()
             .zip(arr.slice(s![1..2, 2..3, ..;2]).iter())
             .all(|(a, b)| a == b));
 
         let mut vi = arr.view();
-        vi.slice_collapse(s![1, 2, NewAxis, 3]);
+        vi.slice_collapse(s![1, 2, 3]);
         assert_eq!(vi.shape(), &[1, 1, 1]);
         assert_eq!(vi, Array3::from_elem((1, 1, 1), arr[(1, 2, 3)]));
     }
 
     // Do it to the ArcArray itself
     let elem = arr[(1, 2, 3)];
     let mut vi = arr;
-    vi.slice_collapse(s![1, 2, 3, NewAxis]);
+    vi.slice_collapse(s![1, 2, 3]);
     assert_eq!(vi.shape(), &[1, 1, 1]);
     assert_eq!(vi, Array3::from_elem((1, 1, 1), elem));
 }
 
+#[test]
+#[should_panic]
+fn test_slice_collapse_with_newaxis() {
+    let mut arr = Array2::<u8>::zeros((2, 3));
+    arr.slice_collapse(s![0, 0, NewAxis]);
+}
+
 #[test]
 fn test_multislice() {
     macro_rules! do_test {