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

Add in/to_slice_memory_order for ArrayView/Mut #1015

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

jturner314
Copy link
Member

This adds a .to_slice_memory_order() method to ArrayView and an .into_slice_memory_order() method to ArrayViewMut. This has unfortunate code duplication with .as_slice_memory_order/_mut(), but the quantity of duplicated code isn't very large.

@@ -221,6 +253,22 @@ where
}
}

/// Return the array’s data as a slice, if it is contiguous.
/// Otherwise return self in the Err branch of the result.
fn try_into_slice_memory_order(self) -> Result<&'a mut [A], Self> {
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's even harder to make consistent, but I think this method is unfortunately generally useful. I've contemplated adding the corresponding one on ArrayBase (where the Err case would be &mut Self). So we should consider if this isn't the best one (or an additional one) to expose for ArrayViewMut.

Copy link
Member Author

Choose a reason for hiding this comment

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

What if we change all the .as_slice*/.to_slice*/.into_slice* methods return a Result? Even if the Err variant isn't especially useful for all the methods, it'll just be a reference (i.e. pointer size) for all but the .into_slice* methods, so it won't cost much.

@bluss bluss modified the milestone: 0.15.3 May 31, 2021
@bluss bluss added this to the 0.15.4 milestone Nov 19, 2021
@bluss
Copy link
Member

bluss commented Nov 22, 2021

Rebased and got the newest spelling from #1119 so we're up to date on that too 😉

@bluss
Copy link
Member

bluss commented Nov 23, 2021

Thanks I'll merge and future improvements can come later - the Result question is left here and it sounds like it could be a good idea! Nice to have the negative tests included here.

@bluss bluss merged commit 7e70a90 into rust-ndarray:master Nov 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants