Skip to content

Test and fix for #143 index with numpy integer item #147

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 3 commits into from
Oct 24, 2017
Merged

Conversation

alimanfoo
Copy link
Member

Resolves #143.

@alimanfoo alimanfoo added this to the v2.2 milestone Apr 26, 2017
zarr/compat.py Outdated
reduce = reduce

else:

text_type = str
binary_type = bytes
integer_types = int,
integer_types = (int,) + numpy_integer_types
Copy link
Member

@jakirkham jakirkham Apr 26, 2017

Choose a reason for hiding this comment

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

Were we using this for conversion or type checking? If the latter, numbers.Integral works well for type checking all integers including NumPy typed ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah OK, didn't know about that. Yes it's for type checking.

@alimanfoo
Copy link
Member Author

Updated to use numbers.Integral. Merging soon.

@jakirkham
Copy link
Member

LGTM. Merge away. :)

@alimanfoo alimanfoo merged commit fa32f1e into master Oct 24, 2017
@alimanfoo alimanfoo deleted the issue_143 branch October 24, 2017 23:33
@alimanfoo
Copy link
Member Author

Thanks :)

@alimanfoo alimanfoo added bug Potential issues with the zarr-python library release notes done Automatically applied to PRs which have release notes. labels Nov 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Potential issues with the zarr-python library release notes done Automatically applied to PRs which have release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants