Skip to content

Tweaks to storage functions for zarr 3 compatibility#61

Draft
SolarDrew wants to merge 2 commits into
asdf-format:mainfrom
SolarDrew:zarr3-updates
Draft

Tweaks to storage functions for zarr 3 compatibility#61
SolarDrew wants to merge 2 commits into
asdf-format:mainfrom
SolarDrew:zarr3-updates

Conversation

@SolarDrew
Copy link
Copy Markdown

No description provided.

@SolarDrew SolarDrew requested a review from a team as a code owner April 27, 2026 10:15
@braingram
Copy link
Copy Markdown
Contributor

Thanks. Would you provide some context for these changes? Do they fix some issue or allow some new feature? Would you add tests that cover this?

Comment thread asdf_zarr/storage.py
for k in _iter_chunk_keys(zarray, only_initialized=True):
index = chunk_key_block_index_map[k]
coords = tuple([int(sk) for sk in k.split(dimension_separator)])
coords = tuple([int(sk) for sk in k.split(dimension_separator)[1:]]) # Don't need the leading 'c/'
Copy link
Copy Markdown
Member

@zacharyburnett zacharyburnett Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it ever possible for this to skip data? can the dimension ever start with a coordinate instead of c/?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I have no idea. I was hoping someone more familiar with the project could tell me. If it turns out it is then obviously I'll make this catch more cases.

@SolarDrew
Copy link
Copy Markdown
Author

SolarDrew commented Apr 27, 2026

@braingram Yes, sorry for springing this up out of nowhere. The fixes are for specific issues I've had trying to serialise a zarray as part of a more complex structure (saving a zarray on its own works just fine I believe). Specifically I need these changes to make this PR work.

I'll probably need to make some more changes as I work through that, so I'll make sure this gets appropriate tests and a better description over the next few days as I get a chance.

@braingram
Copy link
Copy Markdown
Contributor

Thanks. I'll convert this back to draft for now until the other changes are ready.

@braingram braingram marked this pull request as draft April 27, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants