Skip to content

Conversation

LDeakin
Copy link
Member

@LDeakin LDeakin commented Jul 12, 2025

No description provided.

@ilan-gold
Copy link
Collaborator

ilan-gold commented Jul 15, 2025

Looking at the zarr-python code, it seems that this sort of thing is tied to Array creation which is less dynamic than I would have thought i.e., instead of using the write_empty_chunks config value directly from the config itself. But I assume the goal of this PR is to just simplify what we're doing here and move closer to zarr-python if not perfectly aligned?

@d-v-b
Copy link

d-v-b commented Jul 15, 2025

i think eventually in zarr-python we want to support an API where the array config can be locally overridden. The logic for this is that you might want different empty chunks behavior across the domain of the same array. but we haven't pieced together an API for this yet.

@ilan-gold
Copy link
Collaborator

Thanks for the clarification @d-v-b, so then I think this PR seems reasonable then, although https://github.com/zarrs/zarrs-python/pull/103/files#diff-ac216671a3198674cfb77afda777ffa112d2f3c343775adfbbcba98a87216a8fR164-R172 is a little worrying. Is it possible for multiple selections to have different values?

@LDeakin
Copy link
Member Author

LDeakin commented Jul 15, 2025

I picked this up when I was trying to determine whether we wanted ArrayConfig on construction or not for zarr-developers/zarr-python#3233. Our docs on this were incorrect before anyway

@LDeakin LDeakin marked this pull request as ready for review July 15, 2025 12:07
@ilan-gold
Copy link
Collaborator

I guess we couldn't do per-chunk write_empty_chunks anyway given that the CodecOptions are created once and then reused

@LDeakin LDeakin merged commit e7e88ed into main Jul 15, 2025
16 checks passed
@LDeakin LDeakin deleted the ld/dynamic_write_empty_chunks branch July 15, 2025 12:35
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