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

Refactor and improve iterators #502

Merged
merged 14 commits into from Oct 24, 2018

Conversation

jturner314
Copy link
Member

This PR does the following:

  • Removes lots of iterator constructor functions and replaces them with new methods on the iterators. This is more conventional, easier to understand, and requires fewer imports.

  • Renames OuterIterCore to AxisIterCore to better reflect its purpose.

  • Refactors .split_at() for AxisIter and AxisIterMut to eliminate the axis_iter_split_at_impl! macro.

  • Removes the lifetime from Baseiter and implements iterator traits for Baseiter. This is mostly a conceptual simplification. It will also make it more straightforward to extend the iterators to ArrayPtr/ArrayPtrMut in Add raw array pointer types #496.

  • Improve docs and implement Clone for LanesIter.

The old name was misleading because this struct is designed to iterate
over any axis, not just the outermost axis.
This replaces these free functions with `new` methods on `AxisIter`
and `AxisIterMut`.
Removing the macro makes the code easier to understand without being
much more verbose.
It's simpler to treat `Baseiter` as just an iterator over pointers,
with no lifetime information. Lifetimes should be handled as necessary
by the wrappers around `Baseiter`.
@LukeMathWalker
Copy link
Member

From an API point of view this looks great!

@jturner314 jturner314 merged commit 7df47d2 into rust-ndarray:master Oct 24, 2018
@jturner314 jturner314 deleted the refactor-iterators branch October 24, 2018 02:01
@jturner314
Copy link
Member Author

@LukeMathWalker Thanks for taking a look!

@bluss
Copy link
Member

bluss commented Oct 27, 2018

Nice!

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.

None yet

3 participants