fix: make consolidated metadata nesting independent of persisted key order - #4227
Open
d-v-b wants to merge 3 commits into
Open
fix: make consolidated metadata nesting independent of persisted key order#4227d-v-b wants to merge 3 commits into
d-v-b wants to merge 3 commits into
Conversation
…order `ConsolidatedMetadata._flat_to_nested` grouped the flat keys with `itertools.groupby` over keys sorted by depth alone. `groupby` only groups *consecutive* runs, so when a parent's children were not adjacent it emitted several runs for the same parent and the surrounding dict comprehension kept only the last one. Every child in the earlier runs was silently never re-parented, and lingered as a bogus slash-containing key at the top level, making it unreachable through the consolidated metadata. The persisted key order is arbitrary, so nesting must not depend on it. Group by parent with an accumulating mapping instead. This is reachable from zarr-python itself: `to_dict` sorts keys by `(depth, NFKC-casefold(key))`, so sibling subtrees whose names differ only by case interleave and trigger exactly this pattern. Fixes zarr-developers#4226 Assisted-by: ClaudeCode:claude-opus-5
Assisted-by: ClaudeCode:claude-opus-5
Assisted-by: ClaudeCode:claude-opus-5
d-v-b
marked this pull request as ready for review
August 1, 2026 18:51
d-v-b
changed the title
ilan-gold
approved these changes
Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
itertools.groupbystrikes again, this time in our deserialization of consolidated metadata. see d-v-b#273 for the full AI writeup, but basicallyitertertools.groupbyonly groups locally, not globally, so it wasn't gathering all child keys from the flat list of consolidated metadata keys correctly. This PR fixes that behavior by using accumulating into a defaultdict instead of usinggroupby.claude did the diagnosis and the fix. I wrote the complaints about groupby
closes #4226
note that the reproducer in #4226 is also affected by case-insensitive file system issues, which mask the underlying "parse consolidated metadata incorrectly" issue. See d-v-b#273 for the full AI-written explanation.
For reviewers
check that this is correct.
Author attestation
TODO
docs/user-guide/*.mdchanges/