Breaking Change in Fill Value Behavior from 3.0.9 to main for zarr format 2

[ianhi]

Zarr version

3.0.9 vs main

Numcodecs version

0.16.1

Python Version

3.11

Operating System

mac

Installation

uv run

Description

The fill value written for the same inputs has changed from zarr 3.0.9 to main.

With a fill value of None for an int dtype (and time and I think a few others) and zarr format 2, 3.0.9 writes a fill value of null but main writes 0.

I know there are breaking changes in API with dtypes, but this feels like it changes what data is written, without a warning or error. This is causing data corruption errors in the xarray tests for zarr format 2

cc @d-v-b

Steps to reproduce

# /// script
# requires-python = ">=3.11"
# dependencies = [
# "zarr @ git+https://github.com/zarr-developers/zarr-python.git@main"
# ]
# ///

# "zarr==3.0.9",
import json
import os
from pathlib import Path

import numpy as np
import zarr

# zarr.print_debug_info()
print(f"Zarr version: {zarr.__version__}")

store_path = Path(__file__).parent / "test.zarr"

# Create group exactly like xarray does
group = zarr.open_group(store_path, mode="w", zarr_format=2)

# Create array with None fill_value for int64 dtype
array = group.create(
 "test_array", shape=(10,), dtype=np.int64, fill_value=None, chunks="auto"
)

print(f"array.fill_value: {array.fill_value}")
print(f"array.fill_value type: {type(array.fill_value)}")

# Check JSON metadata
with open(os.path.join(store_path, "test_array", ".zarray")) as f:
 metadata = json.load(f)

print(f"JSON fill_value: {metadata['fill_value']}")
print(f"JSON fill_value type: {type(metadata['fill_value'])}")

3.0.9 output

Zarr version: 3.0.9
array.fill_value: None
array.fill_value type: <class 'NoneType'>
JSON fill_value: None
JSON fill_value type: <class 'NoneType'>

main output

Zarr version: 3.0.9.dev43+gbaabf08d
array.fill_value: 0
array.fill_value type: <class 'numpy.int64'>
JSON fill_value: 0
JSON fill_value type: <class 'int'>

Additional output

You might need to run with uv run --no-cache to force uv to reinstall zarr between runs

[d-v-b]

I know there are breaking changes in API with dtypes, but this feels like it changes what data is written, without a warning or error. This is causing data corruption errors in the xarray tests for zarr format 2

I'm surprised the behavior of fill values has changed, since the dtype changes are not in 3.0.9. I'm not sure offhand what is causing this. I'll try to track it down.

This is causing data corruption errors in the xarray tests for zarr format 2

I would recommend that xarray not use tests that depend on the particular default values zarr-python uses. Presumably whatever zarr functionality xarray is testing can be tested in a situation where the fill value is set explicitly, OR the observed data is compared to expected data which both used zarr python's default fill value. But if there is a test that is hard-coding the assumption that zarr python use a particular default fill value for arrays, that's a brittle test.

[ianhi]

I think I have a fix.

[d-v-b]

ah I was confused, I thought you were finding fill value errors in 3.0.9. Now I realize this is about the behavior of zarr v2 fill values after the dtypes changes. The decision to avoid creating null fill values for zarr v2 data was deliberate, based on conversations in this PR -- after those dtype changes, fill_value=None means "use the default fill value for the data type", which is much less ambiguous than a default value of "0" which has to be converted for many dtypes.

The price of this change is that we can no longer specify a null fill value from the top-level array creation routines. I thought this was OK, because people who are motivated can still do so by manually constructing an array metadata document.

Can you explain a bit more why xarray needs to create arrays with a fill value set to null?

[ianhi]

@dcherian understands this better than I do, but I think in short when xarray reads from zarr format 2 it has some complicated behaviors of using the fill value as a mask, instead of the fill value as sparse array. I dont't think this is ideal, but it is this way for valid historical reasons.

I thought this was OK, because people who are motivated can still do so by manually constructing an array metadata document.

This might be ok, what's the easiest way to do this?

However, I guess the worry is that this is a change in behavior without a warning emitted that changes the data that gets written to disk (fortunately only the metadata so recoverable).

[ianhi]

To summarize my understanding here is the behavior of None as a fill value for zarr python version x zarr format:

Version	Format 2	Format 3
3.0.9	null	default scalar
Main	default scalar	default scalar

[d-v-b]

do I understand this correctly: xarray needs to be able to set fill_value=None for zarr v2 data and have the end result be "fill_value": null in .zarray.

In other words, the problem isn't that the default value changed, but rather that it's no longer possible to get arrays with "fill_value": null from the top-level array creation routines?

because if so, I can get the outcome I want by using a different value than None to denote the "default fill value", and leave None available to denote "null fill value" for zarr v2.

[ianhi]

because if so, I can get the outcome I want by using a different value than None to denote the "default fill value", and leave None available to denote "null fill value" for zarr v2.

This would be my preference. It's a bit clunky, but then there is no silent behavior change. Which we could fix in xarray, but it might also break someone else.

Also, we should try to add this to the regression tests. I think they don't fully exercise the fill values otherwise it would have errored on this.

[d-v-b]

to elaborate, I wouild introduce a class like this:

class DefaultFillValue:
 """
 This class does nothing but indicate to zarr python internals that the default fill value should be used.
 """
DEFAULT_FILL_VALUE = DefaultFillValue()

def create_array(..., fill_value=DEFAULT_FILL_VALUE)

[d-v-b]

Also, we should try to add this to the regression tests. I think they don't fully exercise the fill values otherwise it would have errored on this.

This wouldn't quite work with the current regression tests, since they are currently designed such that main produces data, <old version> opens it, resaves it, and main opens it again. That wouldn't catch this kind of regression.

[ianhi]

That solution makes sense to me. Although for zarr format 3 I guess None should also continue to be interpreted as default fill value. Does this mean it's impossible to do top level "null" for fill value in zarr format 3?

I wish python had DEFAULT and NONE. The None = default sometimes and sometimes truly means nothing comes up every so often

[d-v-b]

That solution makes sense to me. Although for zarr format 3 I guess None should also continue to be interpreted as default fill value. Does this mean it's impossible to do top level "null" for fill value in zarr format 3?

I wish python had DEFAULT and NONE. The None = default sometimes and sometimes truly means nothing comes up every so often

in zarr v3 the fill value field must be set. it can't be null, which is an improvement IMO. Presumably xarray handles this already? Which makes me wonder why null is required for v2 only.

And for creating zarr v3 data in zarr python, I think we can keep None around as an alias for DEFAULT_FILL_VALUE, to avoid breaking code.

[ianhi]

in zarr v3 the fill value field must be set. it can't be null, which is an improvement IMO. Presumably xarray handles this already? Which makes me wonder why null is required for v2 only.

yes xarray handles this properly, this is only supporting zarr format 2.

I think we can keep None around as an alias for DEFAULT_FILL_VALUE, to avoid breaking code.

👍 seems best ot me

[dcherian]

Yes, there is a longstanding conceptual error in xarray around handling Zarr V2 fill value.

However, the "breaking" change here is that Zarr V3 does not allow writing a Zarr V2 store with a null fill value. I bet the tests don't catch it because they are all roundtrip tests, and there are no backwards compatibility tests.