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

Fix Zip::indexed for the 0-dimensional case #862

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

jturner314
Copy link
Member

This commit fixes a panic for 0-dimensional, indexed Zip instances
which results from an out-of-bounds index in a call to
IndexPtr::stride_offset in Zip::inner. Basically, the "stride" for
IndexPtr is the axis to update, but for the 0-dimensional case,
there are no axes, so IndexPtr::stride_offset cannot be called
without panicking due to the self.index[stride] access.

The chosen solution is to add a special check to Zip::apply_core for
the 0-dimensional case. Another possible solution would be to modify
the loop of Zip::inner such that an offset would not be performed
for an index of zero.

This commit fixes a panic for 0-dimensional, indexed `Zip` instances
which results from an out-of-bounds index in a call to
`IndexPtr::stride_offset` in `Zip::inner`. Basically, the "stride" for
`IndexPtr` is the axis to update, but for the 0-dimensional case,
there are no axes, so `IndexPtr::stride_offset` cannot be called
without panicking due to the `self.index[stride]` access.

The chosen solution is to add a special check to `Zip::apply_core` for
the 0-dimensional case. Another possible solution would be to modify
the loop of `Zip::inner` such that an offset would not be performed
for an index of zero.
@jturner314 jturner314 added the bug label Dec 14, 2020
@bluss
Copy link
Member

bluss commented Dec 14, 2020

Thanks!

@bluss bluss merged commit cb2dedb into rust-ndarray:master Dec 14, 2020
@jturner314 jturner314 deleted the fix-zip-indexed-0dim branch December 15, 2020 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants