Skip to content

Add map_mut and map_axis_mut #460

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.

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

Merged
merged 7 commits into from
Jun 14, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
64 changes: 60 additions & 4 deletions src/impl_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ impl<A, S, D> ArrayBase<S, D> where S: Data<Elem=A>, D: Dimension
/// **Panics** if any dimension of `chunk_size` is zero<br>
/// (**Panics** if `D` is `IxDyn` and `chunk_size` does not match the
/// number of array axes.)
pub fn exact_chunks<E>(&self, chunk_size: E) -> ExactChunks<A, D>
pub fn exact_chunks<E>(&self, chunk_size: E) -> ExactChunks<A, D>
where E: IntoDimension<Dim=D>,
{
exact_chunks_of(self.view(), chunk_size)
Expand Down Expand Up @@ -930,7 +930,7 @@ impl<A, S, D> ArrayBase<S, D> where S: Data<Elem=A>, D: Dimension
/// [6, 6, 7, 7, 8, 8, 0],
/// [6, 6, 7, 7, 8, 8, 0]]));
/// ```
pub fn exact_chunks_mut<E>(&mut self, chunk_size: E) -> ExactChunksMut<A, D>
pub fn exact_chunks_mut<E>(&mut self, chunk_size: E) -> ExactChunksMut<A, D>
Copy link
Member

Choose a reason for hiding this comment

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

Please avoid formatting changes outside the code you are editing.

Copy link
Member Author

@LukeMathWalker LukeMathWalker Jun 13, 2018

Choose a reason for hiding this comment

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

My vim settings removes trailing whitespaces on save... Got to change it when I am working on ndarray :P

Copy link
Member

Choose a reason for hiding this comment

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

I have Emacs set up the same way. Fwiw, I've found it useful to have the "save" keybinding set to remove trailing whitespace, but preserve the ability to call the save-buffer command directly if desired to save without removing whitespace.

Anyway, I've submitted PR #464 to remove all the trailing whitespace in ndarray so that we won't have to worry about this issue anymore.

For the purpose of this PR, though, @bluss is right that the PR should not reformat lines unrelated to the changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I was thinking about doing something similar to that in my .init.vim.
Until #464 lands I guess I'll be restoring whitespaces once the code has been approved and it is just waiting to be merged - final clean-up, so to speak.

where E: IntoDimension<Dim=D>,
S: DataMut
{
Expand All @@ -941,13 +941,13 @@ impl<A, S, D> ArrayBase<S, D> where S: Data<Elem=A>, D: Dimension
///
/// The windows are all distinct overlapping views of size `window_size`
/// that fit into the array's shape.
///
///
/// Will yield over no elements if window size is larger
/// than the actual array size of any dimension.
///
/// The produced element is an `ArrayView<A, D>` with exactly the dimension
/// `window_size`.
///
///
/// **Panics** if any dimension of `window_size` is zero.<br>
/// (**Panics** if `D` is `IxDyn` and `window_size` does not match the
/// number of array axes.)
Expand Down Expand Up @@ -1694,6 +1694,34 @@ impl<A, S, D> ArrayBase<S, D> where S: Data<Elem=A>, D: Dimension
}
}

/// Call `f` on a mutable reference of each element and create a new array
/// with the new values.
///
/// Elements are visited in arbitrary order.
///
/// Return an array with the same shape as `self`.
pub fn map_mut<'a, B, F>(&'a mut self, f: F) -> Array<B, D>
where F: FnMut(&mut A) -> B,
A: 'a,
Copy link
Member

Choose a reason for hiding this comment

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

The 'a parameter now has no use and can be removed. (It having no use unlike the &self version of map, is correct.)

Copy link
Member

Choose a reason for hiding this comment

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

Hm.. wait a minute, that's not right, using 'a the same way as the &self map would be good.

S: DataMut
{
let dim = self.dim.clone();
let strides = self.strides.clone();
if self.is_contiguous() {
Copy link
Member

Choose a reason for hiding this comment

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

This redundant is_contiguous call is unfortunate. The strides.clone call can move inside the first if branch though, so that's nice.

let slc = self.as_slice_memory_order_mut().unwrap();
let v = ::iterators::to_vec_mapped(slc.iter_mut(), f);
unsafe {
ArrayBase::from_shape_vec_unchecked(
dim.strides(strides), v)
}
} else {
let v = ::iterators::to_vec_mapped(self.iter_mut(), f);
unsafe {
ArrayBase::from_shape_vec_unchecked(dim, v)
}
}
}

/// Call `f` by **v**alue on each element and create a new array
/// with the new values.
///
Expand Down Expand Up @@ -1819,4 +1847,32 @@ impl<A, S, D> ArrayBase<S, D> where S: Data<Elem=A>, D: Dimension
}
})
}

/// Reduce the values along an axis into just one value, producing a new
/// array with one less dimension.
/// 1-dimensional lanes are passed as mutable references to the reducer,
/// allowing for side-effects.
///
/// Elements are visited in arbitrary order.
///
/// Return the result as an `Array`.
///
/// **Panics** if `axis` is out of bounds.
pub fn map_axis_mut<'a, B, F>(&'a mut self, axis: Axis, mut mapping: F)
-> Array<B, D::Smaller>
where D: RemoveAxis,
F: FnMut(ArrayViewMut1<'a, A>) -> B,
A: 'a + Clone,
Copy link
Member

Choose a reason for hiding this comment

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

This A: Clone here is unwanted

Copy link
Member

Choose a reason for hiding this comment

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

The 'a here is good.

S: DataMut,
{
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.subview_mut(axis, 0).map_mut(|first_elt: &mut A| {
unsafe {
mapping(ArrayViewMut::new_(first_elt, Ix1(view_len), Ix1(view_stride)))
}
})
Copy link
Member

Choose a reason for hiding this comment

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

This looks good!

}
}
4 changes: 3 additions & 1 deletion src/iterators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ macro_rules! outer_iter_split_at_impl {
{
assert!(index <= self.iter.len);
let right_ptr = if index != self.iter.len {
unsafe { self.iter.offset(index) }
unsafe { self.iter.offset(index) }
}
else {
self.iter.ptr
Expand Down Expand Up @@ -1180,9 +1180,11 @@ use indexes::IndicesIterF;

unsafe impl<F> TrustedIterator for Linspace<F> { }
unsafe impl<'a, A, D> TrustedIterator for Iter<'a, A, D> { }
unsafe impl<'a, A, D> TrustedIterator for IterMut<'a, A, D> { }
unsafe impl<I, F> TrustedIterator for std::iter::Map<I, F>
where I: TrustedIterator { }
unsafe impl<'a, A> TrustedIterator for slice::Iter<'a, A> { }
unsafe impl<'a, A> TrustedIterator for slice::IterMut<'a, A> { }
unsafe impl TrustedIterator for ::std::ops::Range<usize> { }
// FIXME: These indices iter are dubious -- size needs to be checked up front.
unsafe impl<D> TrustedIterator for IndicesIter<D> where D: Dimension { }
Expand Down