Skip to content

Conversation

@gouttegd
Copy link

The DefaultChunkKeyEncoding constructor currently requires a Configuration argument, which means that the Jackson deserializer will fail upon attempting to parse array metadata containing a chunk_key_encoding like this one:

"chunk_key_encoding": {
  "name": "default"
}

Instead, it expects, at a minimum:

"chunk_key_encoding": {
  "name": "default",
  "configuration": {}
}

I believe Zarr-Java is being too strict here. It is true that that the spec for the "Default chunk key encoding" [1] says nothing about the configuration object being optional (it only says that the separator member is optional). But the spec for extension points [2] (and chunk_key_encoding is an extension point) does say that the configuration member MAY be present (and therefore is optional), and in the absence of any overriding statement, I believe configuration should be considered optional also in the context of the "Default chunk key encoding" extension.

There are definitely OME-Zarr files out in the wild with a missing configuration member, for example [3].

This commit relaxes the requirement in the constructor of both the DefaultChunkKeyEncoding and V2ChunkKeyEncoding classes, so that they accept a missing (null) configuration argument and default to using a / and . separator, respectively.

--
[1] https://zarr-specs.readthedocs.io/en/latest/v3/chunk-key-encodings/default/index.html
[2] https://zarr-specs.readthedocs.io/en/latest/v3/core/index.html#extension-definition
[3] https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.5/idr0066/ExpD_chicken_embryo_MIP.ome.zarr

The DefaultChunkKeyEncoding constructor currently requires a
`Configuration` argument, which means that the Jackson deserializer will
fail upon attempting to parse array metadata containing a
`chunk_key_encoding` like this one:

```
"chunk_key_encoding": {
  "name": "default"
}
```

Instead, it expects, at a minimum:

```
"chunk_key_encoding": {
  "name": "default",
  "configuration": {}
}
```

I believe Zarr-Java is being too strict here. It is true that that the
spec for the "Default chunk key encoding" [1] says nothing about the
`configuration` object being optional (it only says that the `separator`
member is optional). But the spec for extension points [2] (and
`chunk_key_encoding` _is_ an extension point) does say that the
`configuration` member MAY be present (and therefore is optional), and
in the absence of any overriding statement, I believe `configuration`
should be considered optional _also_ in the context of the "Default
chunk key encoding" extension.

There are definitely OME-Zarr files out in the wild with a missing
`configuration` member, for example [3].

This commit relaxes the requirement in the constructor of both the
DefaultChunkKeyEncoding and V2ChunkKeyEncoding classes, so that they
accept a missing (null) configuration argument and default to using a
'/' and '.' separator, respectively.

--
[1] https://zarr-specs.readthedocs.io/en/latest/v3/chunk-key-encodings/default/index.html
[2] https://zarr-specs.readthedocs.io/en/latest/v3/core/index.html#extension-definition
[3] https://uk1s3.embassy.ebi.ac.uk/idr/zarr/v0.5/idr0066/ExpD_chicken_embryo_MIP.ome.zarr
@brokkoli71 brokkoli71 self-requested a review December 18, 2025 17:22
Copy link
Member

@brokkoli71 brokkoli71 left a comment

Choose a reason for hiding this comment

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

Hi @gouttegd,
Thanks for the contribution! This seems like a reasonable relaxation to me

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.

2 participants