Bounds on `remove_index`

rust-ndarray / ndarray

ndarray: an N-dimensional array with array views, multidimensional slicing, and efficient operations

https://docs.rs/ndarray/

Apache License 2.0

3.59k stars 307 forks source link

Bounds on `remove_index` #1442

Open akern40 opened 2 weeks ago

akern40 commented 2 weeks ago

While working on some code, I ran across remove_index and noticed that it has a bound of S: DataOwned, but I'm not sure why?

pub fn remove_index(&mut self, axis: Axis, index: usize)
where S: DataOwned + DataMut

As this method documents, it does not actually change the allocation of the underlying array, and none of its internal methods require DataOwned. Can this bound be removed? Or am I missing something?

bluss commented 1 week ago

That's a good question https://github.com/rust-ndarray/ndarray/pull/967 That guy seems rather confident about it being for owned arrays, but leaves no trace why, I also don't get it. :sweat_smile:

If I try to reconstruct, I think it's a little bit about the mindset and the whole picture.

You start with an owned array and imagine you can remove an index (like removing an element from a vector).
You realize this is only easy do the equivalent of Vec::swap_remove

I think you have a good point, but if we open this method up to being used on all mutable views, then the full effect of the method has to be explained, that it has taken the 'removed index' and placed it somewhere else.

akern40 commented 5 days ago

I can't tell either - even rotate1_front doesn't require DataOwned, and the other functions don't, either. In fact, the method already uses views!

the full effect of the method has to be explained, that it has taken the 'removed index' and placed it somewhere else

We already do explain the full effect: https://github.com/rust-ndarray/ndarray/blob/492b2742073cf531635d701ced4e01a827038f69/src/impl_methods.rs#L2984-L2987