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

Bugfix for Issue 218: NDCube slicing with numpy.int64 #223

Merged
merged 1 commit into from
Dec 28, 2019

Conversation

DanRyanIrish
Copy link
Member

Description

If an NDCube axis after the first axis was sliced with a non-native int type,
e.g. numpy.int64, the slicing would crash due to missing_axes not being
altered. This commit adds support for numpy.int64 and numpy.int32 types.

Fixes #218

@ghost
Copy link

ghost commented Dec 16, 2019

Thanks for the pull request @DanRyanIrish! Everything looks great!

@@ -207,7 +207,7 @@ def _wcs_slicer(wcs, missing_axes, item):
item_ = _slice_list(item_checked)
new_wcs = wcs.slice(item_)
for i, it in enumerate(item_checked):
if isinstance(it, int):
if isinstance(it, (int, np.int64, np.int32)):
Copy link
Member

@Cadair Cadair Dec 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will work

Suggested change
if isinstance(it, (int, np.int64, np.int32)):
if isinstance(it, numbers.Integral):

having done a import numbers

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it does! Thanks!

@DanRyanIrish
Copy link
Member Author

Hi @Cadair. Is there a better way to capture all int-types rather than just listing them out as is done in this PR?

If an axis after the first axis was sliced with a non-native int type,
e.g. numpy.int64, the slixing would crash due to missing_axes not being
altered.  This commit adds support for nupy.int64 and numpy.int32 types.
Copy link
Member

@Cadair Cadair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI fails are unrelated, although they need investigating before a patch release.

Copy link
Member

@tiagopereira tiagopereira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine!

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.

Indexing beyond first dimension does not work for numpy.int types
3 participants