Repository navigation
Conversation
| return len_slices(rf_item) == shape | ||
|
|
||
|
|
||
| def normalize_axis_selection(item, l): |
There was a problem hiding this comment.
This function is no longer needed. That said, I can leave it, change it to use kenjutsu as well, or just remove it. Please let me know what you think.
There was a problem hiding this comment.
Went ahead and tried to restructure things to use this function anyways for validation purposes. That way we can leverage all of the current tests in the first pass.
|
Should add that I saw a lot of constraints on what kind of Though some of them like an overly large |
jakirkham
changed the title
|
Alright, I think this is ready for review. |
|
Thank you, very helpful, and very nice that the new ellipsis tests are passing. I'd like to mull this over for a bit. I've spent some time looking at the I think I may bring simplified versions of these functions into Zarr as an interim solution. That way I can go through every line of code and get acquainted. This is such a critical piece of code I think I need to understand what every line is doing. Also I think it will help to have this functionality inside Zarr when working on fancy indexing, as fancy indexing will require some changes to these functions. But then ultimately when the dust settles from that, maybe consider factoring back out to kenjutsu. |
|
Closing as superseded by indexing work #172. |
Fixes https://github.com/alimanfoo/zarr/issues/93
Uses kenjutsu to handle a variety of tricky cases with Ellipsis, negative step sizes, negative indices, and standard cases as well. This simplifies the code, avoids a variety of bugs. Also this should avoid the need for normalizing each axis, but have not yet cut that code out.