Repository navigation
Fix for nested store path in ZarrParser - #913
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #913 +/- ##
==========================================
- Coverage 89.20% 88.69% -0.51%
==========================================
Files 33 33
Lines 2047 2052 +5
==========================================
- Hits 1826 1820 -6
- Misses 221 232 +11
🚀 New features to boost your workflow:
|
ZarrParser.__call__ was discarding the store-relative path from registry.resolve(), causing two bugs with nested stores: 1. open_group_async couldn't find the zarr group within the store 2. Chunk manifest paths were doubled (e.g. subdir/data.zarr/subdir/data.zarr/temp/0.0) Now correctly uses the store-relative path for group opening and the store root URI for chunk path construction. Note: also requires zarr-developers/zarr-python#3657 (on zarr main, not yet released) which fixes ObjectStore.list_dir corrupting directory names. Co-Authored-By: Claude Opus 4.6 <[email protected]>
a0c61d6 to
cae9d13
Compare
GitHub Actions
Inactive
|
Very weird lithops-related error in the CI here @chuckwondo |
I suspect it may be a transient error since the py311 build passed. I kicked off a rerun of the failed build. If it fails again, I'll dig into it. Either way, I will submit a PR (probably today) that rips out those recent lithops changes I made because the upstream PR I made to fix the issue in lithops itself was already merged and released in lithops 3.6.4 this past weekend. This means I can rip out my workaround and pin lithops in the dev dependencies to be >=3.6.4. |
|
Ah, indeed, the failure appears to have been a transient issue. The 2nd run succeeded. Again, I'll submit a PR to upgrade lithops though, and rip out the workaround, which I believe may reduce the likelihood of this type of error occurring again, since it failed in relation to the code I will end up ripping back out. The fix in lithops itself is simpler and likely more robust than the workaround, so I think it should avoid this in the future. |
|
Thank you @chuckwondo !! |
Co-Authored-By: Claude Opus 4.6 <[email protected]>
GitHub Actions
Inactive
|
Now the CI seems to be hanging... But this fix is right anyway, and @jbusecke needs it, so I'm going to merge. |
Yeah, I've seen this locally using I canceled the workflow so it wouldn't run forever. |
* add empty release notes * Fix ZarrParser when zarr store is nested inside object store root ZarrParser.__call__ was discarding the store-relative path from registry.resolve(), causing two bugs with nested stores: 1. open_group_async couldn't find the zarr group within the store 2. Chunk manifest paths were doubled (e.g. subdir/data.zarr/subdir/data.zarr/temp/0.0) Now correctly uses the store-relative path for group opening and the store root URI for chunk path construction. Note: also requires zarr-developers/zarr-python#3657 (on zarr main, not yet released) which fixes ObjectStore.list_dir corrupting directory names. Co-Authored-By: Claude Opus 4.6 <[email protected]> * Add release note for nested store path fix (zarr-developers#913) Co-Authored-By: Claude Opus 4.6 <[email protected]> --------- Co-authored-by: Claude Opus 4.6 <[email protected]>
Note this requires an unreleased change to zarr-python to work, so should only be merged oncezarr-python=>3.1.6is released, and we should pin to greater than that as part of this PR.Acceptance criteria:
ZarrParserfails when zarr store is nested inside object store root #912docs/releases.mdNew functions/methods are listed in an appropriate*.mdfile underdocs/apiNew functionality has documentation