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

Check expr type for expr;step in s![] macro at compile time, and remove AxisSliceInfo::step_by #943

Merged

Conversation

jturner314
Copy link
Member

Before, if the expr;step notation was used with an index as the expr, e.g. s![4;2], the macro would panic at runtime. Now, it errors at compile time. This is accomplished by converting the expr to a Slice before converting it into the final AxisSliceInfo.

The AxisSliceInfo::step_by method is awkward because it panics for all cases except the Slice variant. This method is no longer necessary because the s![] macro now calls Slice::step_by() instead.

Before, if the `expr;step` notation was used with an index as the
`expr`, e.g. `s![4;2]`, the macro would panic at runtime. Now, it
errors at compile time. This is accomplished by converting the `expr`
to a `Slice` before converting it into the final `AxisSliceInfo`.
@jturner314 jturner314 force-pushed the remove-axissliceinfo-step_by branch 3 times, most recently from c05db1c to 7a20918 Compare March 16, 2021 00:33
This method is awkward because it panics for all cases except the
`Slice` variant. This method is no longer necessary because the `s![]`
macro now calls `Slice::step_by()` instead.
@jturner314 jturner314 added this to the 0.15.0 milestone Mar 16, 2021
@jturner314 jturner314 merged commit b5556f8 into rust-ndarray:master Mar 17, 2021
@jturner314 jturner314 deleted the remove-axissliceinfo-step_by branch March 17, 2021 01:57
@bluss
Copy link
Member

bluss commented Mar 17, 2021

Thanks. Would you like to describe your recent PRs in the change log?

@bluss
Copy link
Member

bluss commented Mar 17, 2021

We can both push to #939 as long as it's open for example. Or just open up for PRs to add to the changelog by themselves.

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