Skip to content

Use TryFrom over From #147

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 8 commits into from
Oct 13, 2021
Merged

Conversation

mulimoen
Copy link
Collaborator

@mulimoen mulimoen commented Apr 10, 2021

This changes the From implementations of SliceOrIndex/Hyperslab/Selection to TryFrom.

SliceOrIndex is now reworked to only accept parameters resulting in valid selections. The old code for working with negative strides in selections to allow python like negative indexing has been removed, which removed a lot of the special casing to allow for this.

A future addition could be to introduce a hyperslab macro which mirrors ndarray::s, but allows the size of the block to be selected.

Fixes #142

@mulimoen mulimoen force-pushed the feature/try_into_selection branch 2 times, most recently from 2df478f to c9b52c7 Compare April 17, 2021 10:35
@mulimoen mulimoen marked this pull request as ready for review April 17, 2021 10:40
@mulimoen mulimoen mentioned this pull request May 19, 2021
@mulimoen mulimoen force-pushed the feature/try_into_selection branch from 70054de to cc89b2d Compare May 20, 2021 18:07
@mulimoen mulimoen force-pushed the feature/try_into_selection branch from cc89b2d to 43ee34d Compare August 17, 2021 08:35
@mulimoen mulimoen force-pushed the feature/try_into_selection branch 2 times, most recently from 972e402 to 5ffa3f9 Compare October 6, 2021 19:59
@aldanor
Copy link
Owner

aldanor commented Oct 13, 2021

@mulimoen Any changes/updates to this, do we want to rebase/merge it? (should changelog be updated then?)

@mulimoen mulimoen force-pushed the feature/try_into_selection branch from 5ffa3f9 to 252136f Compare October 13, 2021 16:17
@mulimoen
Copy link
Collaborator Author

This change is orthogonal to all other PRs. There is need for changes in the CHANGELOG, this overwrites the unreleased Extents and friends

@aldanor
Copy link
Owner

aldanor commented Oct 13, 2021

Yea, make sense, I've misread it. Could you rebase (just in case)? We'll wait till it's green and then merge

@mulimoen
Copy link
Collaborator Author

@aldanor Rebased and green

@aldanor
Copy link
Owner

aldanor commented Oct 13, 2021

👍

@aldanor aldanor merged commit b762e22 into aldanor:master Oct 13, 2021
@mulimoen mulimoen deleted the feature/try_into_selection branch October 13, 2021 17:26
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

Successfully merging this pull request may close these issues.

Remove potential panics in From impl
2 participants