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.

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

Bounds on remove_index #1442

Open
akern40 opened this issue Oct 12, 2024 · 2 comments
Open

Bounds on remove_index #1442

akern40 opened this issue Oct 12, 2024 · 2 comments

Comments

@akern40
Copy link
Collaborator

akern40 commented Oct 12, 2024

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
Copy link
Member

bluss commented Oct 24, 2024

That's a good question #967
That guy seems rather confident about it being for owned arrays, but leaves no trace why, I also don't get it. 😅

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
Copy link
Collaborator Author

akern40 commented Oct 26, 2024

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:

ndarray/src/impl_methods.rs

Lines 2984 to 2987 in 492b274

/// Note that this "removes" the elements by swapping them around to the end of the axis and
/// shortening the length of the axis; the elements are not deinitialized or dropped by this,
/// just moved out of view (this only matters for elements with ownership semantics). It's
/// similar to slicing an owned array in place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants