Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support zero-length axis in .map_axis/_mut() #612

Merged
merged 6 commits into from
Apr 9, 2019
Merged

Support zero-length axis in .map_axis/_mut() #612

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
89ebe56
Support zero-length `axis` in `.map_axis/_mut()`
Mar 29, 2019
e121948
Moved empty ArrayView[Mut] creation to new method
Mar 30, 2019
e30762d
Changed the implementation
Apr 2, 2019
7d6d0ef
Made the test not to rely on `map_axis` iteration order
Apr 3, 2019
b39a0bb
Merge pull request #1 from rust-ndarray/master
Apr 3, 2019
04e9bad
Merge branch 'master' of github.com:andrei-papou/ndarray into zero-le…
Apr 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 24 additions & 14 deletions src/impl_methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2066,13 +2066,18 @@ where
{
let view_len = self.len_of(axis);
let view_stride = self.strides.axis(axis);
// use the 0th subview as a map to each 1d array view extended from
// the 0th element.
self.index_axis(axis, 0).map(|first_elt| {
unsafe {
mapping(ArrayView::new_(first_elt, Ix1(view_len), Ix1(view_stride)))
}
})
if view_len == 0 {
let new_dim = self.dim.remove_axis(axis);
Array::from_shape_fn(new_dim, move |_| mapping(ArrayView::from(&[])))
} else {
// use the 0th subview as a map to each 1d array view extended from
// the 0th element.
self.index_axis(axis, 0).map(|first_elt| {
unsafe {
mapping(ArrayView::new_(first_elt, Ix1(view_len), Ix1(view_stride)))
}
})
}
}

/// Reduce the values along an axis into just one value, producing a new
Expand All @@ -2094,12 +2099,17 @@ where
{
let view_len = self.len_of(axis);
let view_stride = self.strides.axis(axis);
// use the 0th subview as a map to each 1d array view extended from
// the 0th element.
self.index_axis_mut(axis, 0).map_mut(|first_elt: &mut A| {
unsafe {
mapping(ArrayViewMut::new_(first_elt, Ix1(view_len), Ix1(view_stride)))
}
})
if view_len == 0 {
let new_dim = self.dim.remove_axis(axis);
Array::from_shape_fn(new_dim, move |_| mapping(ArrayViewMut::from(&mut [])))
} else {
// use the 0th subview as a map to each 1d array view extended from
// the 0th element.
self.index_axis_mut(axis, 0).map_mut(|first_elt| {
unsafe {
mapping(ArrayViewMut::new_(first_elt, Ix1(view_len), Ix1(view_stride)))
}
})
}
}
}
29 changes: 29 additions & 0 deletions tests/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2002,6 +2002,35 @@ fn test_map_axis() {
let c = a.map_axis(Axis(1), |view| view.sum());
let answer2 = arr1(&[6, 15, 24, 33]);
assert_eq!(c, answer2);

// Test zero-length axis case
let arr = Array3::<f32>::zeros((3, 0, 4));
let mut counter = 0;
let result = arr.map_axis(Axis(1), |x| {
assert_eq!(x.shape(), &[0]);
counter += 1;
counter
});
assert_eq!(result.shape(), &[3, 4]);
assert_eq!(result, arr2(&[
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, this test is good, but we don't guarantee the iteration order of map_axes, so the elements of result are not guaranteed to be arranged in this way. Something like

itertools::assert_equal(result.iter().cloned().sorted(), 1..=3*4);

would work to verify that the elements are correct while ignoring their ordering.

(For access to .sorted(), make sure to use itertools::Itertools and add features = ["use_std"] to the itertools dependency in Cargo.toml.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've updated the test accordingly.

[1, 2, 3, 4],
[5, 6, 7, 8],
[9, 10, 11, 12]
]));

let mut arr = Array3::<f32>::zeros((3, 0, 4));
let mut counter = 0;
let result = arr.map_axis_mut(Axis(1), |x| {
assert_eq!(x.shape(), &[0]);
counter += 1;
counter
});
assert_eq!(result.shape(), &[3, 4]);
assert_eq!(result, arr2(&[
[1, 2, 3, 4],
[5, 6, 7, 8],
[9, 10, 11, 12]
]));
}

#[test]
Expand Down