Skip to content

Rework subview methods and other related methods #537

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 9 commits into from
Nov 19, 2018

Conversation

jturner314
Copy link
Member

This PR does the following:

  • Rename subview_inplace to collapse_axis (deprecating the old name).
  • Rename subview_mut toindex_axis_mut (deprecating the old name).
  • Rename into_subview to index_axis_move (deprecating the old name).
  • Rename private function do_sub to do_collapse_axis.
  • Rename subview to index_axis (deprecating the old name).
  • Improve docs.
  • Rename slice_inplace to slice_collapse (deprecating the old name).
  • Add insert_axis_inplace and index_axis_inplace for IxDyn arrays.
  • Deprecate remove_axis.

These changes make method names more uniform, especially between subviews and slicing, and provide new functionality for IxDyn arrays.

Possible alternative names for "collapse" include "narrow" and "restrict".

After a while with slice_inplace deprecated, we can remove it, then add a slice_inplace method for IxDyn arrays that removes axes like slice_move does.

What do you think?

@bluss
Copy link
Member

bluss commented Nov 12, 2018

Nice reorganization.

@bluss
Copy link
Member

bluss commented Nov 12, 2018

Great. I don't have the release overview at the moment, if we have breaking changes in flight already or not. Is it time to start merging them? I can make point releases before then if we need to.

@jturner314
Copy link
Member Author

I don't have the release overview at the moment, if we have breaking changes in flight already or not.

I just submitted #540, which describes the changes between 0.12.0 and the current master. We haven't merged any breaking changes yet.

@jturner314
Copy link
Member Author

One point of interest about the way I've implemented insert_axis_inplace and index_axis_inplace in this PR: the dim and strides are temporarily inconsistent. This is fine, though, even if self.strides.insert_axis(axis) or self.strides.remove_axis(axis) panics, because nothing can observe the inconsistency since ArrayBase doesn't implement Drop and &mut ArrayBase<S, D> is !UnwindSafe.

On a related note, I'm currently in the process of clarifying the invariants that methods on ArrayBase must uphold (and when the invariants need to be upheld) and updating the array creation methods to enforce the invariants. (This will also fix a couple of edge cases that currently result in undefined behavior.) I'll submit a PR when I'm done.

@jturner314
Copy link
Member Author

I just rebased the commits off the latest master and updated the array0_into_scalar test added in #535.

@jturner314
Copy link
Member Author

Is this ready to merge? Everything looks good to me.

@bluss
Copy link
Member

bluss commented Nov 19, 2018

Sure!

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