valid identifiers for the `bytes` codec

[d-v-b]

My understanding is that there are five possible JSON declarations for the bytes codec:

1. "bytes"
2. {"name": "bytes"}
3. {"name": "bytes", "configuration": {}}
4. {"name": "bytes", "configuration": {"endian": "little"}}
5. {"name": "bytes", "configuration": {"endian": "big"}}

First, am I correct in this census or is there a detail from the spec I am missing?

Second, unless I'm misreading the spec, we have defined 3 different ways of saying the exact same thing (items 1-3). Putting aside the merits of this design, I'm curious what the different zarr v3 implementations support. cc @LDeakin @jbms

right now zarr-python only supports options 2 - 5. See the script and results below.

demo script

# /// script
# requires-python = ">=3.11"
# dependencies = [
#   "zarr @ git+https://github.com/zarr-developers/zarr-python.git@27615fd",
#   "pytest"
# ]
# ///
from typing import Any
import zarr
import pytest 

from zarr.core.metadata.v3 import ArrayV3Metadata

bytes_codec_specs = [
    "bytes",
    {"name": "bytes"},
    {"name": "bytes", "configuration": {}},
    {"name": "bytes", "configuration": {"endian": "little"}},
    {"name": "bytes", "configuration": {"endian": "big"}},
]

@pytest.mark.parametrize('bytes_codec_spec', bytes_codec_specs, ids=[str(x) for x in bytes_codec_specs])
def test(bytes_codec_spec: str | dict[str, Any]) -> None:
    data = {
        "node_type": "array",
        "fill_value": 0,
        "zarr_format": 3,
        "shape": [100, 100, 3],
        "data_type": "uint8",
        "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [10, 10, 3]}},
        "chunk_key_encoding": {"name": "default", "configuration": {"separator": "/"}},
        "codecs": [bytes_codec_spec]
        }
    ArrayV3Metadata.from_dict(data)

if __name__ == "__main__":
    pytest.main([__file__, f'-c {__file__}'])

test.py3.11-1.25-minimalbennettd@dvb-desktop-0 ➜  zarr-python git:(feat/numcodecs-compat) ✗ uv run test.py
Reading inline script metadata from `test.py`
 Updated https://github.com/zarr-developers/zarr-python.git (27615fd0)
Cannot read termcap database;
using dumb terminal settings.
============================================================================================================================================= test session starts =============================================================================================================================================
platform linux -- Python 3.11.5, pytest-8.4.1, pluggy-1.6.0
rootdir: /home/bennettd/dev/zarr-python/ /home/bennettd/dev/zarr-python
configfile: test.py
collected 5 items                                                                                                                                                                                                                                                                                             

 /home/bennettd/dev/zarr-python F....                                                                                                                                                                                                                                                                   [100%]

================================================================================================================================================== FAILURES ===================================================================================================================================================
_________________________________________________________________________________________________________________________________________________ test[bytes] _________________________________________________________________________________________________________________________________________________

bytes_codec_spec = 'bytes'

    @pytest.mark.parametrize('bytes_codec_spec', bytes_codec_specs, ids=[str(x) for x in bytes_codec_specs])
    def test(bytes_codec_spec: str | dict[str, Any]) -> None:
        data = {
            "node_type": "array",
            "fill_value": 0,
            "zarr_format": 3,
            "shape": [100, 100, 3],
            "data_type": "uint8",
            "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [10, 10, 3]}},
            "chunk_key_encoding": {"name": "default", "configuration": {"separator": "/"}},
            "codecs": [bytes_codec_spec]
            }
>       ArrayV3Metadata.from_dict(data)

test.py:34: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../.cache/uv/archive-v0/DL3T_HZF_GHlo1WkQHjiQ/lib/python3.11/site-packages/zarr/core/metadata/v3.py:323: in from_dict
    return cls(**_data, fill_value=fill_value_parsed, data_type=data_type)  # type: ignore[arg-type]
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
../../.cache/uv/archive-v0/DL3T_HZF_GHlo1WkQHjiQ/lib/python3.11/site-packages/zarr/core/metadata/v3.py:177: in __init__
    codecs_parsed_partial = parse_codecs(codecs)
                            ^^^^^^^^^^^^^^^^^^^^
../../.cache/uv/archive-v0/DL3T_HZF_GHlo1WkQHjiQ/lib/python3.11/site-packages/zarr/core/metadata/v3.py:66: in parse_codecs
    name_parsed, _ = parse_named_configuration(c, require_configuration=False)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

data = 'bytes', expected_name = None

    def parse_named_configuration(
        data: JSON, expected_name: str | None = None, *, require_configuration: bool = True
    ) -> tuple[str, JSON | None]:
        if not isinstance(data, dict):
>           raise TypeError(f"Expected dict, got {type(data)}")
E           TypeError: Expected dict, got <class 'str'>

../../.cache/uv/archive-v0/DL3T_HZF_GHlo1WkQHjiQ/lib/python3.11/site-packages/zarr/core/common.py:130: TypeError
=========================================================================================================================================== short test summary info ===========================================================================================================================================
FAILED  /home/bennettd/dev/zarr-python::test[bytes] - TypeError: Expected dict, got <class 'str'>
========================================================================================================================================= 1 failed, 4 passed in 0.08s =========================================================================================================================================

[LDeakin]

zarrs supports all of them for reading. https://docs.rs/zarrs_metadata/latest/zarrs_metadata/v3/struct.MetadataV3.html#example-metadata. However, codecs are always serialised using variant 2-5 since zarrs/zarrs@acaad11.

Fixed

Fixed a regression introduced in v0.11.2 (89fc63f) where codecs with an empty configuration would serialise as a string rather than a struct with a name field, which goes against the zarr spec

Based on that changelog, variant 1 was previously not allowed. Is there a way to look at a rendered version of the spec before 3.1?

[d-v-b]

the change occurred here (see my comment)

[LDeakin]

There is also another variant since Zarr 3.1: {"name": "bytes", "configuration": {}, "must_understand": true}.

I technically support this in zarrs and would like to start writing "must_understand": false with a codec like bitround, but I'm waiting for ecosystem support.

[d-v-b]

is there a discussion somewhere about "must_understand": False in the context of codecs?

[LDeakin]

I'm fairly certain this was discussed in an issue/PR on GitHub, but I can't find it. Maybe @normanrz mentioned it?

The spec permits it:

An extension object MAY explicitly set must_understand=False if implementations can ignore its presence.

must_understand=False is not supported for the following extension points: data type, chunk grid, and chunk key encoding.

[jbms]

Variant 1 is not allowed by the spec. Tensorstore allows that as a shorthand in a tensorstore json spec but not when reading stored metadata.

[d-v-b]

Variant 1 is not allowed by the spec.

per https://zarr-specs.readthedocs.io/en/latest/v3/core/index.html#array-metadata-codecs:

Each codec is an extension point and MUST conform to the Extension definition.

which links a section that contains this text:

Instead of extension objects, short-hand names MAY be used if no configuration metadata is required. They are equivalent to extension objects with just a name key.

[jbms]

Variant 1 is not allowed by the spec.

per https://zarr-specs.readthedocs.io/en/latest/v3/core/index.html#array-metadata-codecs:

Each codec is an extension point and MUST conform to the Extension definition.

which links a section that contains this text:

Instead of extension objects, short-hand names MAY be used if no configuration metadata is required. They are equivalent to extension objects with just a name key.

Oh, thanks for correcting me. I'll have to fix that in tensorstore. Not sure where I got the idea that the shorthand wasn't allowed for codecs.

[d-v-b]

fwiw i'm not sure that this change was intentional, as I don't recall it being discussed in #330, so a quick reversion would also be possible, provided none of the existing implementations are writing the shorthand form of codecs.

If we disallow "bytes", that would still leave three redundant forms:

1. "bytes"
2. {"name": "bytes"}
3. {"name": "bytes", "configuration": {}}
4. {"name": "bytes", "configuration": {}, "must_understand": true}

To keep things tidy I think we should coordinate on which form we write. I would note that "coordinate the behavior of implementations" is the job of a spec, so the spec is letting us down a bit here by allowing these redundant forms of the same thing.

[LDeakin]

I don't think it should be reverted. It is a sensible shorthand and it is consistent with other array metadata fields.

I think it is a valid change given the Zarr stability policy anyway. Variant 1 and 4 would just be considered a "feature" of Zarr 3.1. Implementations can favor variant 2 over 1 if they wish to maintain compatibility with Zarr 3.0 implementations. It'd be handy to have a note in the spec about this.

[d-v-b]

It'd be handy to have a note in the spec about this.

It would also be helpful if the bytes codec spec enumerated the valid JSON forms

[d-v-b]

As it's evidently not obvious that 3.1 included this change, we should ensure that everyone working on zarr v3 implementations knows about it. I think that includes @bogovicj and @WardF, and maybe other people I'm forgetting right now.