You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Error in string data type handling with 0 fill value #2792
There are Zarr V2 string arrays in the wild with a 0 fill value. zarr-python interprets this as an empty string (e.g. when partially writing a chunk), but a completely missing chunk returns 0's rather than ""s. See reproducer.
where did these zarr arrays come from? Unfortunately the v2 spec doesn't associate the fill_value field with a concrete type, so it's not possible to state with authority that a JSON number is an invalid fill value for an array of strings, but I feel like a fill value stored as the json number 0 does not unambiguously map to the empty string "". Is that what people expect here?
and in the context of anndata, what is the expected interpretation of 0 (a JSON number) as a fill value for an array of strings? Is it intended to be interpreted as the string "0"?
Perhaps zarr-python should restore the whole str(fill_value) behaviour with zarr_format=2 string arrays for compatibility, and just reject non-string fill values for zarr_format=3?
if the intention is for the fill value to be the string "0", then it seems like anndata.write_zarr should be setting the fill value to a string instead of a number.
On zarr-python's side, my personal preference would be to view "dtype is string, but fill value is a JSON number" as an error in metadata parsing, and we should produce a nice error message that advises people on how to fix their metadata documents.
That might be a bit drastic given our history of permissiveness here, so we maybe we start by doing what you recommend @LDeakin: casting JSON numbers to strings, and emitting a warning if this is necessary, with the promise that in a few releases fill values will be handled more strictly.
we should produce a nice error message that advises people on how to fix their metadata documents.
If I understand this correctly, basically all of anndata's i/o would break with all previously created data until users change their files directly? I am not so sure that's a good idea.
casting JSON numbers to strings, and emitting a warning if this is necessary, with the promise that in a few releases fill values will be handled more strictly.
I also am not so sure about this for similar reasons. Why not just continue the backwards compatibility for v2, and for v3 do things correctly? At the minimum, for reading (if you want to prevent people from writing bad data, that is fine).
No that is just for merging arrays like anndata.concat([my_anndata, her_anndata])
Looking at our codebase, there is not mention at all of fill_value outside of the concatenation context.
I will look into why this is happening, but I am strongly opposed to breaking anndata's read capabilities contingent on users changing a realtively-obscure metadata field in an outdated file format
So this would break compat with anyone who has written a string array like this in the past, which I imagine is more than us. I think a fix for the v3 package that preserves the read behavior but disables writing arrays with the wrong fill-value is reasonable.
so if this is due to a bad default from zarr-python 2.x, then we should keep that behavior in zarr-python 3 to minimize discomfort. I'm not even sure if we should change how we write new zarr v2 data, since zarr v2 readers might be assuming the fill value for string arrays will be a JSON number?
Activity
d-v-b commentedon Feb 5, 2025
where did these zarr arrays come from? Unfortunately the v2 spec doesn't associate the
fill_valuefield with a concrete type, so it's not possible to state with authority that a JSON number is an invalid fill value for an array of strings, but I feel like a fill value stored as the json number0does not unambiguously map to the empty string "". Is that what people expect here?LDeakin commentedon Feb 5, 2025
scverse/anndata-rs#15 @zqfang?
Yeah you are right. It differs between
zarr-pythonversions:['a' 'bb' '' '0' '0']['a' 'bb' '' '' 0]It looks like the old behaviour was indeed to just
str()the fill valueszqfang commentedon Feb 6, 2025
from the anndata.write_zarr()
d-v-b commentedon Feb 6, 2025
and in the context of anndata, what is the expected interpretation of
0(a JSON number) as a fill value for an array of strings? Is it intended to be interpreted as the string"0"?LDeakin commentedon Feb 6, 2025
@ilan-gold? I think it might have just been overlooked.
anndatahas used a default fill value of0for every data type for a long time https://github.com/scverse/anndata/blame/8d7beab49d04f5c1d91c847e0e1af99795d4d25f/src/anndata/_core/merge.py#L723-L738.Perhaps
zarr-pythonshould restore the wholestr(fill_value)behaviour withzarr_format=2string arrays for compatibility, and just reject non-string fill values forzarr_format=3?d-v-b commentedon Feb 6, 2025
if the intention is for the fill value to be the string
"0", then it seems likeanndata.write_zarrshould be setting the fill value to a string instead of a number.On zarr-python's side, my personal preference would be to view "dtype is string, but fill value is a JSON number" as an error in metadata parsing, and we should produce a nice error message that advises people on how to fix their metadata documents.
That might be a bit drastic given our history of permissiveness here, so we maybe we start by doing what you recommend @LDeakin: casting JSON numbers to strings, and emitting a warning if this is necessary, with the promise that in a few releases fill values will be handled more strictly.
ilan-gold commentedon Feb 7, 2025
If I understand this correctly, basically all of anndata's i/o would break with all previously created data until users change their files directly? I am not so sure that's a good idea.
I also am not so sure about this for similar reasons. Why not just continue the backwards compatibility for v2, and for v3 do things correctly? At the minimum, for reading (if you want to prevent people from writing bad data, that is fine).
No that is just for merging arrays like
anndata.concat([my_anndata, her_anndata])Looking at our codebase, there is not mention at all of
fill_valueoutside of the concatenation context.I will look into why this is happening, but I am strongly opposed to breaking anndata's read capabilities contingent on users changing a realtively-obscure metadata field in an outdated file format
ilan-gold commentedon Feb 7, 2025
This is default zarr v2 (i.e., the package) behavior:
So this would break compat with anyone who has written a string array like this in the past, which I imagine is more than us. I think a fix for the v3 package that preserves the read behavior but disables writing arrays with the wrong fill-value is reasonable.
d-v-b commentedon Feb 7, 2025
so if this is due to a bad default from zarr-python 2.x, then we should keep that behavior in zarr-python 3 to minimize discomfort. I'm not even sure if we should change how we write new zarr v2 data, since zarr v2 readers might be assuming the fill value for string arrays will be a JSON number?
ilan-gold commentedon Feb 7, 2025
That's a fair point. I am not sure I'd expect packages to lack parsing for the correct metadata but it's possible.
LDeakin commentedon Feb 8, 2025
Ok full investigation of the old behaviour (zarr-python 2.18.4):
Summarising:
dtype=strhas"0"fill values 🥲dtype=object, object_codec=numcodecs.VLenUTF8()has a0fill value that maps to""on diskNone/nullfill value always maps to""on diskSo
zarr-python3.x.x just needs to add backwards compatibility support for the second case for Zarr V2 data.d-v-b commentedon Sep 24, 2026
I think this was completed via #2874, closing!