-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Hard-code .zmetadata in FSStore._normalize_key #1467
Conversation
The normalization code in FSStore uses a list of well-known files to prevent incorrectly re-writing keys when dimension separator is "/" rather than ".". This was initialized only with the names specified in the spec, which left out ".zmetadata" from consolidated metadata. see: zarr-developers#1121
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1467 +/- ##
=======================================
Coverage 99.99% 99.99%
=======================================
Files 38 38
Lines 14574 14574
=======================================
Hits 14573 14573
Misses 1 1
|
It would be good to have a failing test in the suite before this gets merged. |
self._array_meta_key, | ||
self._group_meta_key, | ||
self._attrs_key, | ||
".zmetadata", # see: #1121 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we do this via a class attribute like _consolidated_meta_key
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and that would definitely be better than my quick fix here (intended to test the CI) but there's really no good place for it to live. It should naturally be in a Consolidated*
class, but FSStore
should be unaware of that. This is simply a problem of there being no spec for consolidated.
Any thoughts about getting this into the 2.17.x line? |
I'm going to close this as stale. Folks should feel free to reopen if there is interest in continuing this work. |
The normalization code in FSStore uses a list of well-known files to prevent incorrectly re-writing keys when dimension separator is "/" rather than ".". This was initialized only with the names specified in the spec, which left out ".zmetadata" from consolidated metadata.
see: #1121
TODO: