Skip to content

Ellipsis handling #168

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.

Sign up for GitHub

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

Jump to bottom
Closed
alimanfoo wants to merge 2 commits into from
Closed

Ellipsis handling #168

alimanfoo wants to merge 2 commits into from

Conversation

@alimanfoo
Copy link
Member

@alimanfoo alimanfoo commented Oct 25, 2017 •
edited
Loading

This PR has tests and minimal fix for bug in handling of indexing with ellipsis (#93). Also deals with case where too many index items are provided than there are dimensions. Resolves #93. Also resolves #89.

@alimanfoo
Copy link
Member Author

alimanfoo commented Oct 25, 2017

cc @jakirkham. I know you have an open PR (#116) that resolves #93 via kenjutsu, but I'd like to keep the indexing code within zarr for the time being, so have made this PR with a minimal fix for #93. That said, if you have any comments on the implementation here, and/or advice about any other tricky indexing cases we should be covering, that would be very welcome. (I'm going to investigate fancy indexing in a separate branch.)

@alimanfoo alimanfoo added this to the v2.2 milestone Oct 25, 2017
@jakirkham
Copy link
Member

jakirkham commented Oct 25, 2017

Not a problem. I think that PR got a bit stale. Plus there were some open questions about performance. So a simpler near term fix is reasonable and desirable.

Happy to take a look. Won't be able to do so right away, but will try to set aside some time this weekend or some time next week.

@alimanfoo alimanfoo changed the title Ellipsis handling; resolves #93 Ellipsis handling Oct 31, 2017
@alimanfoo
Copy link
Member Author

alimanfoo commented Nov 10, 2017

Closing as these changes have been rolled into #172.

@alimanfoo alimanfoo closed this Nov 10, 2017
@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
@jhamman jhamman deleted the indexing-20171025 branch October 15, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug Potential issues with the zarr-python library release notes done Automatically applied to PRs which have release notes.

Projects

None yet

Milestone

v2.2

Development

Successfully merging this pull request may close these issues.

BUG: Parsing ellipsis in indexing Better error message if attempt fancy indexing

3 participants

@alimanfoo @jakirkham