8000 Add into_scalar for ArrayView0 and ArrayViewMut0 by LukeMathWalker · Pull Request #700 · rust-ndarray/ndarray · GitHub
[go: up one dir, main page]

Skip to content

Add into_scalar for ArrayView0 and ArrayViewMut0#700

Merged
LukeMathWalker merged 7 commits intorust-ndarray:masterfrom
LukeMathWalker:into-scalar
Sep 8, 2019
Merged

Add into_scalar for ArrayView0 and ArrayViewMut0#700
LukeMathWalker merged 7 commits intorust-ndarray:masterfrom
LukeMathWalker:into-scalar

Conversation

@LukeMathWalker
Copy link
Member

This addresses #688.

I have taken the chance to re-organize impl_views, converting it into a folder sub-module to then split it into smaller files with a functionality-related name.
I think this makes it easier to navigate the crate (and we should probably do the same for other very big files we have in the repository) but I am ok to revert the changes if you believe otherwise.

/// let scalar: &Foo = view.into_scalar();
/// assert_eq!(scalar, &Foo);
/// ```
pub fn into_scalar(self) -> &'a A {
Copy link
Member
@bluss bluss Sep 8, 2019

Choose a reason for hiding this comment

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

the ArrayView doesn't need to be consumed to allow this (infuriating difference between shared and mut for IndexLonger). I guess one could just as well make this a &self method, it makes a difference for non-copy ArrayViews?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about it, but then I opted to keep the signature consistent with the other two (Array/ArrayViewMut). Given that it's named into_*, it sounded preferable to consume the input.
On the other hand, taking &self makes it more convenient for ArrayView because you can keep using self afterwards.

I don't know, I would lean on the side of symmetry in this case.

Copy link
Member

Choose a reason for hiding this comment

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

ok, that works.

@bluss
Copy link
Member
bluss commented Sep 8, 2019

Looks good. Continual splitting is good I think. Impl_views isn't even large in ndarray 😄

@LukeMathWalker LukeMathWalker merged commit e4d2711 into rust-ndarray:master Sep 8, 2019
@LukeMathWalker LukeMathWalker deleted the into-scalar branch September 8, 2019 17:19
@bluss
Copy link
Member
bluss commented Sep 8, 2019

Tip, use the https://help.github.com/en/articles/closing-issues-using-keywords names. With "Fixes #688." it closes the issue automatically. I prefer to put these in the PR text (not in the commit log because rebasing and re-pushing causes spam on the issue).

@LukeMathWalker
Copy link
Member Author

Nifty trick, thanks!

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

0