Fix indexing bug when reading past the first chunk in a shard
Closes #2018 by passing the correct format for the byte_range argument to a StorePath byte getter when accessing chunks within a shard. Previously, the return value of get_chunk_slice was directly passed in as the byte range. This is incorrect since it is formatted as (start index, end index) when it should be (start index, total read length).
TODO:
- [x] Add unit tests and/or doctests in docstrings
- [ ] Add docstrings and API docs for any new/modified user-facing classes and functions
- [ ] New/modified features documented in docs/tutorial.rst
- [ ] Changes documented in docs/release.rst
- [ ] GitHub Actions have all passed
- [ ] Test coverage is 100% (Codecov passes)
Thanks! I don't quite understand why this hasn't shown up in the tests. Would you mind adding a test for this bug to https://github.com/zarr-developers/zarr-python/blob/v3/tests/v3/test_codecs/test_sharding.py?
Taking a quick glance at the test suite indicates that we never attempt to read an intermediate slice outside of the first chunk in a shard. Any access beyond the first chunk is always a slice without an upper bound.
I could either modify the existing tests to include accesses the fit the pattern in the bug, or I can add a separate test case. What would you prefer?
Thanks! I think adding parameterization for multiple access patterns is the way to go. That is what your commit attempts, right?
Yeah that's what the commit does.
The test failure of a segfault is puzzling, but I believe it is genuine. ~~I'm using my PR branch in another codebase, and I'm also seeing a segfault there occurring around the same point.~~ (ignore this data point) Any idea what it could be?