From e6b6130becc585ebef02f43023dcfbda8a3832e9 Mon Sep 17 00:00:00 2001 From: Anish Mehta Date: Thu, 24 Sep 2026 07:22:24 +0530 Subject: [PATCH 1/5] fix: accept must_understand=false extra fields in v3 group metadata Closes #3523 --- changes/3523.bugfix.md | 1 + src/zarr/core/group.py | 17 +++++++++++++++++ tests/test_group.py | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 changes/3523.bugfix.md diff --git a/changes/3523.bugfix.md b/changes/3523.bugfix.md new file mode 100644 index 0000000000..d285b4c6ae --- /dev/null +++ b/changes/3523.bugfix.md @@ -0,0 +1 @@ +Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise `MetadataValidationError` instead of a `TypeError`. diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index d734e6b7cd..85524d282f 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -49,6 +49,7 @@ from zarr.core.json_parse import parse_field from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata from zarr.core.metadata.io import save_metadata +from zarr.core.metadata.v3 import AllowedExtraField, check_allowed_extra_field from zarr.core.sync import SyncMixin, sync from zarr.errors import ( ArrayNotFoundError, @@ -355,6 +356,7 @@ class GroupMetadata(Metadata): zarr_format: ZarrFormat = 3 consolidated_metadata: ConsolidatedMetadata | None = None node_type: Literal["group"] = field(default="group", init=False) + extra_fields: dict[str, AllowedExtraField] = field(default_factory=dict) def to_buffer_dict(self, prototype: BufferPrototype) -> dict[str, Buffer]: indent = config.get("json_indent") @@ -406,6 +408,7 @@ def __init__( attributes: dict[str, Any] | None = None, zarr_format: ZarrFormat = 3, consolidated_metadata: ConsolidatedMetadata | None = None, + extra_fields: dict[str, AllowedExtraField] | None = None, ) -> None: attributes_parsed = parse_attributes(attributes) zarr_format_parsed = parse_zarr_format(zarr_format) @@ -413,6 +416,7 @@ def __init__( object.__setattr__(self, "attributes", attributes_parsed) object.__setattr__(self, "zarr_format", zarr_format_parsed) object.__setattr__(self, "consolidated_metadata", consolidated_metadata) + object.__setattr__(self, "extra_fields", dict(extra_fields or {})) @classmethod def from_dict(cls, data: dict[str, Any]) -> GroupMetadata: @@ -433,11 +437,24 @@ def from_dict(cls, data: dict[str, Any]) -> GroupMetadata: # extra key in the metadata. expected = {x.name for x in fields(cls)} data = {k: v for k, v in data.items() if k in expected} + else: + # Zarr v3 allows an extra key only if it is an object with "must_understand": false. + known = {"attributes", "zarr_format", "consolidated_metadata"} + extra = {k: data.pop(k) for k in set(data) - known} + invalid = sorted(k for k, v in extra.items() if not check_allowed_extra_field(v)) + if invalid: + raise MetadataValidationError( + f"Got a Zarr V3 group metadata document with disallowed extra fields: {invalid}. " + 'Extra fields are only allowed if they are an object with "must_understand" ' + "set to false." + ) + data["extra_fields"] = extra return cls(**data) def to_dict(self) -> dict[str, Any]: result = asdict(replace(self, consolidated_metadata=None)) + result.update(result.pop("extra_fields")) if self.consolidated_metadata is not None: result["consolidated_metadata"] = self.consolidated_metadata.to_dict() else: diff --git a/tests/test_group.py b/tests/test_group.py index 31fbd138cd..8e7f76a78e 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -1724,6 +1724,39 @@ def test_from_dict_extra_fields(self): expected = GroupMetadata(attributes={"key": "value"}, zarr_format=2) assert result == expected + def test_from_dict_v3_allowed_extra_fields(self) -> None: + # https://github.com/zarr-developers/zarr-python/issues/3523 + extension = {"must_understand": False, "version": 1} + data = {"zarr_format": 3, "node_type": "group", "my_extension": extension} + result = GroupMetadata.from_dict(data) + assert result.extra_fields == {"my_extension": extension} + assert result.to_dict() == {**data, "attributes": {}} + + @pytest.mark.parametrize("value", [{"must_understand": True}, {}, "not an object", 1]) + def test_from_dict_v3_disallowed_extra_fields(self, value: object) -> None: + data = {"zarr_format": 3, "node_type": "group", "my_extension": value} + with pytest.raises(MetadataValidationError, match="my_extension"): + GroupMetadata.from_dict(data) + + def test_extra_fields_survive_rewrite(self) -> None: + store = MemoryStore() + extension = {"must_understand": False} + doc = {"zarr_format": 3, "node_type": "group", "attributes": {}, "ext": extension} + sync( + store.set( + "zarr.json", default_buffer_prototype().buffer.from_bytes(json.dumps(doc).encode()) + ) + ) + + group = zarr.open_group(store=store, mode="r+") + group.attrs["key"] = "value" + + written = json.loads( + sync(store.get("zarr.json", prototype=default_buffer_prototype())).to_bytes() + ) + assert written["ext"] == extension + assert written["attributes"] == {"key": "value"} + class TestInfo: def test_info(self): From e9ae1fc637f44f3ba512edb9d0aedce8f18572e7 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Tue, 29 Sep 2026 18:49:38 +0200 Subject: [PATCH 2/5] fix(group): match array extra_fields validation and refuse v2 extras Share the extra-field checks between array and group metadata: `parse_extra_fields` takes the reserved keys, and the new `extract_extra_fields` splits and validates extra keys in `from_dict`. `GroupMetadata` now rejects extra fields that collide with its own keys, refuses extra fields on Zarr format 2 groups, and no longer passes a v2 `.zgroup` key named `extra_fields` to its constructor. Rename the changelog fragment to the PR number. Assisted-by: ClaudeCode:claude-opus-5-5 --- changes/{3523.bugfix.md => 4391.bugfix.md} | 2 +- src/zarr/core/group.py | 57 ++++++++++------- src/zarr/core/metadata/v3.py | 72 +++++++++++++++------- tests/test_group.py | 56 ++++++++++++----- 4 files changed, 126 insertions(+), 61 deletions(-) rename changes/{3523.bugfix.md => 4391.bugfix.md} (60%) diff --git a/changes/3523.bugfix.md b/changes/4391.bugfix.md similarity index 60% rename from changes/3523.bugfix.md rename to changes/4391.bugfix.md index d285b4c6ae..579b656dc5 100644 --- a/changes/3523.bugfix.md +++ b/changes/4391.bugfix.md @@ -1 +1 @@ -Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise `MetadataValidationError` instead of a `TypeError`. +Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise `MetadataValidationError` instead of a `TypeError`. `GroupMetadata` takes these fields as `extra_fields`, which rejects keys reserved by the group metadata document and is not supported for Zarr format 2 groups. diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index 85524d282f..cb2555b954 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -5,9 +5,9 @@ import unicodedata import warnings from collections import defaultdict -from dataclasses import asdict, dataclass, field, fields, replace +from dataclasses import asdict, dataclass, field, replace from itertools import accumulate -from typing import TYPE_CHECKING, Literal, assert_never, cast, overload +from typing import TYPE_CHECKING, Final, Literal, assert_never, cast, overload import numpy as np @@ -49,7 +49,7 @@ from zarr.core.json_parse import parse_field from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata from zarr.core.metadata.io import save_metadata -from zarr.core.metadata.v3 import AllowedExtraField, check_allowed_extra_field +from zarr.core.metadata.v3 import AllowedExtraField, extract_extra_fields, parse_extra_fields from zarr.core.sync import SyncMixin, sync from zarr.errors import ( ArrayNotFoundError, @@ -346,6 +346,17 @@ def flatten( return metadata +GROUP_METADATA_KEYS: Final[set[str]] = { + "zarr_format", + "node_type", + "attributes", + "consolidated_metadata", +} +""" +The names of the fields of the group metadata document written by Zarr-Python. +""" + + @dataclass(frozen=True) class GroupMetadata(Metadata): """ @@ -408,15 +419,22 @@ def __init__( attributes: dict[str, Any] | None = None, zarr_format: ZarrFormat = 3, consolidated_metadata: ConsolidatedMetadata | None = None, - extra_fields: dict[str, AllowedExtraField] | None = None, + extra_fields: Mapping[str, AllowedExtraField] | None = None, ) -> None: attributes_parsed = parse_attributes(attributes) zarr_format_parsed = parse_zarr_format(zarr_format) + extra_fields_parsed = parse_extra_fields( + extra_fields, reserved_keys=GROUP_METADATA_KEYS, node_type="group" + ) + if zarr_format_parsed == 2 and extra_fields_parsed: + raise ValueError( + "Invalid extra fields. Zarr format 2 group metadata does not support extra fields." + ) object.__setattr__(self, "attributes", attributes_parsed) object.__setattr__(self, "zarr_format", zarr_format_parsed) object.__setattr__(self, "consolidated_metadata", consolidated_metadata) - object.__setattr__(self, "extra_fields", dict(extra_fields or {})) + object.__setattr__(self, "extra_fields", extra_fields_parsed) @classmethod def from_dict(cls, data: dict[str, Any]) -> GroupMetadata: @@ -431,26 +449,19 @@ def from_dict(cls, data: dict[str, Any]) -> GroupMetadata: data["consolidated_metadata"] = ConsolidatedMetadata.from_dict(consolidated_metadata) zarr_format = data.get("zarr_format") - if zarr_format == 2 or zarr_format is None: + if zarr_format == 3: + # Zarr v3 allows an extra key only if it is an object with "must_understand": false. + extra_fields = extract_extra_fields( + data, reserved_keys=GROUP_METADATA_KEYS, node_type="group" + ) + else: # zarr v2 allowed arbitrary keys here. # We don't want the GroupMetadata constructor to fail just because someone put an - # extra key in the metadata. - expected = {x.name for x in fields(cls)} - data = {k: v for k, v in data.items() if k in expected} - else: - # Zarr v3 allows an extra key only if it is an object with "must_understand": false. - known = {"attributes", "zarr_format", "consolidated_metadata"} - extra = {k: data.pop(k) for k in set(data) - known} - invalid = sorted(k for k, v in extra.items() if not check_allowed_extra_field(v)) - if invalid: - raise MetadataValidationError( - f"Got a Zarr V3 group metadata document with disallowed extra fields: {invalid}. " - 'Extra fields are only allowed if they are an object with "must_understand" ' - "set to false." - ) - data["extra_fields"] = extra - - return cls(**data) + # extra key in the metadata, but we don't keep those keys either. An invalid + # zarr_format is rejected by the constructor. + extra_fields = {} + data = {k: v for k, v in data.items() if k in GROUP_METADATA_KEYS} + return cls(**data, extra_fields=extra_fields) def to_dict(self) -> dict[str, Any]: result = asdict(replace(self, consolidated_metadata=None)) diff --git a/src/zarr/core/metadata/v3.py b/src/zarr/core/metadata/v3.py index 11f3eb593d..53ad066c4a 100644 --- a/src/zarr/core/metadata/v3.py +++ b/src/zarr/core/metadata/v3.py @@ -2,6 +2,7 @@ import json from collections.abc import Iterable, Mapping, Sequence +from collections.abc import Set as AbstractSet from dataclasses import dataclass, field, replace from typing import TYPE_CHECKING, Any, Final, Literal, NotRequired, TypeGuard, cast @@ -24,6 +25,7 @@ DimensionNamesLike, NamedConfig, NamedRequiredConfig, + NodeType, compress_rle, expand_rle, parse_named_configuration, @@ -155,8 +157,8 @@ def parse_storage_transformers(data: object) -> tuple[dict[str, JSON], ...]: class AllowedExtraField(TypedDict, extra_items=JSON): # type: ignore[call-arg] """ - This class models allowed extra fields in array metadata. - They must have ``must_understand`` set to ``False``, and may contain + This class models allowed extra fields in array or group metadata. + They must have `must_understand` set to `False`, and may contain arbitrary additional JSON data. """ @@ -173,23 +175,61 @@ def check_allowed_extra_field(data: object) -> TypeGuard[AllowedExtraField]: def parse_extra_fields( data: Mapping[str, AllowedExtraField] | None, + *, + reserved_keys: AbstractSet[str], + node_type: NodeType, ) -> dict[str, AllowedExtraField]: + """ + Check that no extra field key collides with a key reserved by the metadata document. + """ if data is None: return {} else: - conflict_keys = ARRAY_METADATA_KEYS & set(data.keys()) + conflict_keys = reserved_keys & set(data.keys()) if len(conflict_keys) > 0: msg = ( "Invalid extra fields. " "The following keys: " f"{sorted(conflict_keys)} " "are invalid because they collide with keys reserved for use by the " - "array metadata document." + f"{node_type} metadata document." ) raise ValueError(msg) return dict(data) +def extract_extra_fields( + data: Mapping[str, object], + *, + reserved_keys: AbstractSet[str], + node_type: NodeType, +) -> dict[str, AllowedExtraField]: + """ + Return the members of a Zarr V3 metadata document whose keys are not in `reserved_keys`. + + Raises `MetadataValidationError` if any of them is not an object with + `"must_understand": false`. + """ + allowed_extra_fields: dict[str, AllowedExtraField] = {} + invalid_extra_fields: list[str] = [] + for key, val in data.items(): + if key in reserved_keys: + continue + if check_allowed_extra_field(val): + allowed_extra_fields[key] = val + else: + invalid_extra_fields.append(key) + if len(invalid_extra_fields) > 0: + msg = ( + f"Got a Zarr V3 {node_type} metadata document with the following disallowed extra " + f"fields: {sorted(invalid_extra_fields)}. " + 'Extra fields are not allowed unless they are an object with a "must_understand" ' + "key which is assigned the value `false`." + ) + raise MetadataValidationError(msg) + return allowed_extra_fields + + # JSON type for a single dimension's rectilinear spec: # bare int (uniform shorthand), or list of ints / [value, count] RLE pairs. RectilinearDimSpecJSON = int | list[int | list[int]] @@ -518,7 +558,9 @@ def __init__( attributes_parsed = parse_attributes(attributes) codecs_parsed_partial = parse_codecs(codecs) storage_transformers_parsed = parse_storage_transformers(storage_transformers) - extra_fields_parsed = parse_extra_fields(extra_fields) + extra_fields_parsed = parse_extra_fields( + extra_fields, reserved_keys=ARRAY_METADATA_KEYS, node_type="array" + ) array_spec = ArraySpec( shape=shape_parsed, dtype=data_type, @@ -652,23 +694,9 @@ def from_dict(cls, data: dict[str, JSON]) -> Self: raise TypeError(f"Invalid fill_value: {fill!r}") from e # check if there are extra keys - extra_keys = set(_data.keys()) - ARRAY_METADATA_KEYS - allowed_extra_fields: dict[str, AllowedExtraField] = {} - invalid_extra_fields = {} - for key in extra_keys: - val = _data[key] - if check_allowed_extra_field(val): - allowed_extra_fields[key] = val - else: - invalid_extra_fields[key] = val - if len(invalid_extra_fields) > 0: - msg = ( - "Got a Zarr V3 metadata document with the following disallowed extra fields:" - f"{sorted(invalid_extra_fields.keys())}." - 'Extra fields are not allowed unless they are a dict with a "must_understand" key' - "which is assigned the value `False`." - ) - raise MetadataValidationError(msg) + allowed_extra_fields = extract_extra_fields( + _data, reserved_keys=ARRAY_METADATA_KEYS, node_type="array" + ) # TODO: replace this with a real type check! _data_typed = cast(ArrayMetadataJSON_V3, _data) diff --git a/tests/test_group.py b/tests/test_group.py index 8e7f76a78e..1b7e40335a 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -28,6 +28,7 @@ from zarr.core.dtype.common import unpack_dtype_json from zarr.core.dtype.npy.int import UInt8 from zarr.core.group import ( + GROUP_METADATA_KEYS, ConsolidatedMetadata, GroupMetadata, ImplicitGroupMarker, @@ -1718,19 +1719,31 @@ def test_from_dict_extra_fields(self): data = { "attributes": {"key": "value"}, "_nczarr_superblock": {"version": "2.0.0"}, + # a key named like the `extra_fields` parameter is dropped, not passed to __init__ + "extra_fields": {"my_extension": {"must_understand": False}}, "zarr_format": 2, } result = GroupMetadata.from_dict(data) expected = GroupMetadata(attributes={"key": "value"}, zarr_format=2) assert result == expected - def test_from_dict_v3_allowed_extra_fields(self) -> None: + @pytest.mark.parametrize( + "extra_fields", + [ + {}, + {"my_extension": {"must_understand": False, "version": 1}}, + {"a": {"must_understand": False}, "b": {"must_understand": False, "data": [1, 2]}}, + {"extra_fields": {"must_understand": False}}, + ], + ) + def test_v3_extra_fields_round_trip(self, extra_fields: dict[str, Any]) -> None: # https://github.com/zarr-developers/zarr-python/issues/3523 - extension = {"must_understand": False, "version": 1} - data = {"zarr_format": 3, "node_type": "group", "my_extension": extension} + data = {"zarr_format": 3, "node_type": "group", "attributes": {}, **extra_fields} + expected = GroupMetadata(extra_fields=extra_fields) result = GroupMetadata.from_dict(data) - assert result.extra_fields == {"my_extension": extension} - assert result.to_dict() == {**data, "attributes": {}} + assert result == expected + assert result.extra_fields == extra_fields + assert result.to_dict() == data @pytest.mark.parametrize("value", [{"must_understand": True}, {}, "not an object", 1]) def test_from_dict_v3_disallowed_extra_fields(self, value: object) -> None: @@ -1738,24 +1751,37 @@ def test_from_dict_v3_disallowed_extra_fields(self, value: object) -> None: with pytest.raises(MetadataValidationError, match="my_extension"): GroupMetadata.from_dict(data) + @pytest.mark.parametrize("key", sorted(GROUP_METADATA_KEYS)) + def test_init_extra_fields_collision(self, key: str) -> None: + extra_fields: dict[str, Any] = {key: {"must_understand": False}} + with pytest.raises(ValueError, match="collide with keys reserved"): + GroupMetadata(extra_fields=extra_fields) + + def test_init_extra_fields_v2(self) -> None: + with pytest.raises(ValueError, match="Zarr format 2 group metadata does not support"): + GroupMetadata(zarr_format=2, extra_fields={"my_extension": {"must_understand": False}}) + + @pytest.mark.filterwarnings("ignore:Consolidated metadata is:zarr.errors.ZarrUserWarning") def test_extra_fields_survive_rewrite(self) -> None: - store = MemoryStore() extension = {"must_understand": False} doc = {"zarr_format": 3, "node_type": "group", "attributes": {}, "ext": extension} - sync( - store.set( - "zarr.json", default_buffer_prototype().buffer.from_bytes(json.dumps(doc).encode()) - ) - ) + store = MemoryStore() + for key in ("zarr.json", "child/zarr.json"): + buffer = default_buffer_prototype().buffer.from_bytes(json.dumps(doc).encode()) + sync(store.set(key, buffer)) group = zarr.open_group(store=store, mode="r+") - group.attrs["key"] = "value" + group["child"].attrs["key"] = "value" + zarr.consolidate_metadata(store) written = json.loads( - sync(store.get("zarr.json", prototype=default_buffer_prototype())).to_bytes() + sync(store.get("child/zarr.json", prototype=default_buffer_prototype())).to_bytes() ) - assert written["ext"] == extension - assert written["attributes"] == {"key": "value"} + assert written == {**doc, "attributes": {"key": "value"}} + consolidated = zarr.open_consolidated(store) + assert consolidated.metadata.extra_fields == {"ext": extension} + assert consolidated["child"].metadata.extra_fields == {"ext": extension} + assert consolidated["child"].attrs["key"] == "value" class TestInfo: From 6ce30fcb1f5bb3a322690dce2591e82cd6a0e3e8 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Tue, 29 Sep 2026 19:16:04 +0200 Subject: [PATCH 3/5] fix(metadata): reject non-conforming extra fields in v3 metadata constructors `parse_extra_fields` now also checks that every extra field is an object with `"must_understand": false`, so `ArrayV3Metadata` and `GroupMetadata` can no longer be constructed with extra fields that their own `from_dict` would reject. `extract_extra_fields` delegates to it, leaving one place that validates extra fields. The Zarr format 2 group refusal is checked first so it keeps its own message. Assisted-by: ClaudeCode:claude-opus-5-5 --- changes/4391.bugfix.md | 2 +- src/zarr/core/group.py | 8 ++--- src/zarr/core/metadata/v3.py | 64 ++++++++++++++++++---------------- tests/test_group.py | 5 +++ tests/test_metadata/test_v3.py | 17 +++++++++ 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/changes/4391.bugfix.md b/changes/4391.bugfix.md index 579b656dc5..ee3a545f39 100644 --- a/changes/4391.bugfix.md +++ b/changes/4391.bugfix.md @@ -1 +1 @@ -Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise `MetadataValidationError` instead of a `TypeError`. `GroupMetadata` takes these fields as `extra_fields`, which rejects keys reserved by the group metadata document and is not supported for Zarr format 2 groups. +Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise `MetadataValidationError` instead of a `TypeError`. `GroupMetadata` takes these fields as `extra_fields`, which rejects keys reserved by the group metadata document and is not supported for Zarr format 2 groups. The `GroupMetadata` and `ArrayV3Metadata` constructors now reject extra fields that are not objects with `"must_understand": false`, like reading a metadata document does. diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index cb2555b954..b74483cd1f 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -423,13 +423,13 @@ def __init__( ) -> None: attributes_parsed = parse_attributes(attributes) zarr_format_parsed = parse_zarr_format(zarr_format) - extra_fields_parsed = parse_extra_fields( - extra_fields, reserved_keys=GROUP_METADATA_KEYS, node_type="group" - ) - if zarr_format_parsed == 2 and extra_fields_parsed: + if zarr_format_parsed == 2 and extra_fields: raise ValueError( "Invalid extra fields. Zarr format 2 group metadata does not support extra fields." ) + extra_fields_parsed = parse_extra_fields( + extra_fields, reserved_keys=GROUP_METADATA_KEYS, node_type="group" + ) object.__setattr__(self, "attributes", attributes_parsed) object.__setattr__(self, "zarr_format", zarr_format_parsed) diff --git a/src/zarr/core/metadata/v3.py b/src/zarr/core/metadata/v3.py index 53ad066c4a..2c10ef9447 100644 --- a/src/zarr/core/metadata/v3.py +++ b/src/zarr/core/metadata/v3.py @@ -174,54 +174,39 @@ def check_allowed_extra_field(data: object) -> TypeGuard[AllowedExtraField]: def parse_extra_fields( - data: Mapping[str, AllowedExtraField] | None, + data: Mapping[str, object] | None, *, reserved_keys: AbstractSet[str], node_type: NodeType, ) -> dict[str, AllowedExtraField]: """ - Check that no extra field key collides with a key reserved by the metadata document. + Check the extra fields of a Zarr V3 metadata document. + + Raises `ValueError` if a key collides with a key in `reserved_keys`, and + `MetadataValidationError` if a value is not an object with `"must_understand": false`. """ if data is None: return {} - else: - conflict_keys = reserved_keys & set(data.keys()) - if len(conflict_keys) > 0: - msg = ( - "Invalid extra fields. " - "The following keys: " - f"{sorted(conflict_keys)} " - "are invalid because they collide with keys reserved for use by the " - f"{node_type} metadata document." - ) - raise ValueError(msg) - return dict(data) - - -def extract_extra_fields( - data: Mapping[str, object], - *, - reserved_keys: AbstractSet[str], - node_type: NodeType, -) -> dict[str, AllowedExtraField]: - """ - Return the members of a Zarr V3 metadata document whose keys are not in `reserved_keys`. - - Raises `MetadataValidationError` if any of them is not an object with - `"must_understand": false`. - """ + conflict_keys = reserved_keys & set(data.keys()) + if len(conflict_keys) > 0: + msg = ( + "Invalid extra fields. " + "The following keys: " + f"{sorted(conflict_keys)} " + "are invalid because they collide with keys reserved for use by the " + f"{node_type} metadata document." + ) + raise ValueError(msg) allowed_extra_fields: dict[str, AllowedExtraField] = {} invalid_extra_fields: list[str] = [] for key, val in data.items(): - if key in reserved_keys: - continue if check_allowed_extra_field(val): allowed_extra_fields[key] = val else: invalid_extra_fields.append(key) if len(invalid_extra_fields) > 0: msg = ( - f"Got a Zarr V3 {node_type} metadata document with the following disallowed extra " + f"Got Zarr V3 {node_type} metadata with the following disallowed extra " f"fields: {sorted(invalid_extra_fields)}. " 'Extra fields are not allowed unless they are an object with a "must_understand" ' "key which is assigned the value `false`." @@ -230,6 +215,23 @@ def extract_extra_fields( return allowed_extra_fields +def extract_extra_fields( + data: Mapping[str, object], + *, + reserved_keys: AbstractSet[str], + node_type: NodeType, +) -> dict[str, AllowedExtraField]: + """ + Return the checked extra fields of a Zarr V3 metadata document: the members whose keys + are not in `reserved_keys`. See `parse_extra_fields`. + """ + return parse_extra_fields( + {k: v for k, v in data.items() if k not in reserved_keys}, + reserved_keys=reserved_keys, + node_type=node_type, + ) + + # JSON type for a single dimension's rectilinear spec: # bare int (uniform shorthand), or list of ints / [value, count] RLE pairs. RectilinearDimSpecJSON = int | list[int | list[int]] diff --git a/tests/test_group.py b/tests/test_group.py index 1b7e40335a..0dfa6ac922 100644 --- a/tests/test_group.py +++ b/tests/test_group.py @@ -1751,6 +1751,11 @@ def test_from_dict_v3_disallowed_extra_fields(self, value: object) -> None: with pytest.raises(MetadataValidationError, match="my_extension"): GroupMetadata.from_dict(data) + @pytest.mark.parametrize("value", [{"must_understand": True}, {}, "not an object", 1]) + def test_init_disallowed_extra_fields(self, value: object) -> None: + with pytest.raises(MetadataValidationError, match="my_extension"): + GroupMetadata(extra_fields={"my_extension": value}) # type: ignore[dict-item] + @pytest.mark.parametrize("key", sorted(GROUP_METADATA_KEYS)) def test_init_extra_fields_collision(self, key: str) -> None: extra_fields: dict[str, Any] = {key: {"must_understand": False}} diff --git a/tests/test_metadata/test_v3.py b/tests/test_metadata/test_v3.py index 9f78ed7b70..be2564a570 100644 --- a/tests/test_metadata/test_v3.py +++ b/tests/test_metadata/test_v3.py @@ -312,6 +312,23 @@ def test_array_metadata_extra_fields_rejected(case: ExpectFail[dict[str, Any]]) ArrayV3Metadata.from_dict(case.input) +@pytest.mark.parametrize("value", [{"must_understand": True}, {}, 42]) +def test_init_extra_fields_disallowed(value: object) -> None: + """Extra field values that are not objects with `must_understand: false` are rejected.""" + with pytest.raises(MetadataValidationError, match="disallowed extra fields"): + ArrayV3Metadata( + shape=(10,), + data_type=UInt8(), + chunk_grid={"name": "regular", "configuration": {"chunk_shape": (10,)}}, + chunk_key_encoding={"name": "default", "configuration": {"separator": "/"}}, + fill_value=0, + codecs=({"name": "bytes", "configuration": {"endian": "little"}},), + attributes={}, + dimension_names=None, + extra_fields={"my_ext": value}, # type: ignore[dict-item] + ) + + def test_init_extra_fields_collision() -> None: """Extra field keys that collide with reserved metadata field names are rejected.""" extra_fields: dict[str, object] = {"shape": (10,), "data_type": "uint8"} From 6ba0b105b71a15e5a3b9eeff8387af6b3c6022e3 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Tue, 29 Sep 2026 19:26:36 +0200 Subject: [PATCH 4/5] docs(changes): number the changelog fragment for #4451 Assisted-by: ClaudeCode:claude-opus-5-5 --- changes/{4391.bugfix.md => 4451.bugfix.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changes/{4391.bugfix.md => 4451.bugfix.md} (100%) diff --git a/changes/4391.bugfix.md b/changes/4451.bugfix.md similarity index 100% rename from changes/4391.bugfix.md rename to changes/4451.bugfix.md From adbe9e805535ff095272547199cd7c6d77da1278 Mon Sep 17 00:00:00 2001 From: Davis Vann Bennett Date: Tue, 29 Sep 2026 22:51:30 +0200 Subject: [PATCH 5/5] refactor(metadata): inline the extra-field split into from_dict `extract_extra_fields` filtered the reserved keys and then called `parse_extra_fields`, whose collision check could never fire from that path. Both `from_dict`s now do the filter inline and call `parse_extra_fields` directly, so one function owns the extra-field contract. Also tighten the changelog fragment, the `GROUP_METADATA_KEYS` docstring, and the stale repr comments. Assisted-by: ClaudeCode:claude-fable-5-1 --- changes/4451.bugfix.md | 2 +- src/zarr/core/group.py | 22 +++++++++++----------- src/zarr/core/metadata/v3.py | 23 ++++------------------- src/zarr/core/sync_group.py | 2 +- 4 files changed, 17 insertions(+), 32 deletions(-) diff --git a/changes/4451.bugfix.md b/changes/4451.bugfix.md index ee3a545f39..7d4c00a0f9 100644 --- a/changes/4451.bugfix.md +++ b/changes/4451.bugfix.md @@ -1 +1 @@ -Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and keeps them when the metadata is written back. Other extra fields raise `MetadataValidationError` instead of a `TypeError`. `GroupMetadata` takes these fields as `extra_fields`, which rejects keys reserved by the group metadata document and is not supported for Zarr format 2 groups. The `GroupMetadata` and `ArrayV3Metadata` constructors now reject extra fields that are not objects with `"must_understand": false`, like reading a metadata document does. +Zarr format 3 group metadata now accepts extra fields that are objects with `"must_understand": false`, as the specification requires, and writes them back unchanged; previously opening such a group raised `TypeError`. Extra fields of any other shape raise `MetadataValidationError`, both when reading a metadata document and when constructing `GroupMetadata` or `ArrayV3Metadata` directly. `GroupMetadata` exposes these fields as `extra_fields`, like `ArrayV3Metadata`. diff --git a/src/zarr/core/group.py b/src/zarr/core/group.py index b74483cd1f..304cab10b8 100644 --- a/src/zarr/core/group.py +++ b/src/zarr/core/group.py @@ -49,7 +49,7 @@ from zarr.core.json_parse import parse_field from zarr.core.metadata import ArrayV2Metadata, ArrayV3Metadata from zarr.core.metadata.io import save_metadata -from zarr.core.metadata.v3 import AllowedExtraField, extract_extra_fields, parse_extra_fields +from zarr.core.metadata.v3 import AllowedExtraField, parse_extra_fields from zarr.core.sync import SyncMixin, sync from zarr.errors import ( ArrayNotFoundError, @@ -312,9 +312,9 @@ def flattened_metadata(self) -> dict[str, ArrayV2Metadata | ArrayV3Metadata | Gr "group-1": GroupMetadata(), } ) - # {'group-0': GroupMetadata(attributes={}, zarr_format=3, consolidated_metadata=None, node_type='group'), - # 'group-0/group-0-0': GroupMetadata(attributes={}, zarr_format=3, consolidated_metadata=None, node_type='group'), - # 'group-1': GroupMetadata(attributes={}, zarr_format=3, consolidated_metadata=None, node_type='group')} + # {'group-0': GroupMetadata(attributes={}, zarr_format=3, consolidated_metadata=None, node_type='group', extra_fields={}), + # 'group-0/group-0-0': GroupMetadata(attributes={}, zarr_format=3, consolidated_metadata=None, node_type='group', extra_fields={}), + # 'group-1': GroupMetadata(attributes={}, zarr_format=3, consolidated_metadata=None, node_type='group', extra_fields={})} ``` """ metadata = {} @@ -353,7 +353,8 @@ def flatten( "consolidated_metadata", } """ -The names of the fields of the group metadata document written by Zarr-Python. +The names of the fields of the group metadata document. `consolidated_metadata` is not in the +Zarr V3 spec; Zarr-Python writes it. """ @@ -451,14 +452,13 @@ def from_dict(cls, data: dict[str, Any]) -> GroupMetadata: zarr_format = data.get("zarr_format") if zarr_format == 3: # Zarr v3 allows an extra key only if it is an object with "must_understand": false. - extra_fields = extract_extra_fields( - data, reserved_keys=GROUP_METADATA_KEYS, node_type="group" + extra_fields = parse_extra_fields( + {k: v for k, v in data.items() if k not in GROUP_METADATA_KEYS}, + reserved_keys=GROUP_METADATA_KEYS, + node_type="group", ) else: - # zarr v2 allowed arbitrary keys here. - # We don't want the GroupMetadata constructor to fail just because someone put an - # extra key in the metadata, but we don't keep those keys either. An invalid - # zarr_format is rejected by the constructor. + # Zarr v2 allowed arbitrary keys; they are dropped rather than kept. extra_fields = {} data = {k: v for k, v in data.items() if k in GROUP_METADATA_KEYS} return cls(**data, extra_fields=extra_fields) diff --git a/src/zarr/core/metadata/v3.py b/src/zarr/core/metadata/v3.py index 2c10ef9447..2959cadd74 100644 --- a/src/zarr/core/metadata/v3.py +++ b/src/zarr/core/metadata/v3.py @@ -215,23 +215,6 @@ def parse_extra_fields( return allowed_extra_fields -def extract_extra_fields( - data: Mapping[str, object], - *, - reserved_keys: AbstractSet[str], - node_type: NodeType, -) -> dict[str, AllowedExtraField]: - """ - Return the checked extra fields of a Zarr V3 metadata document: the members whose keys - are not in `reserved_keys`. See `parse_extra_fields`. - """ - return parse_extra_fields( - {k: v for k, v in data.items() if k not in reserved_keys}, - reserved_keys=reserved_keys, - node_type=node_type, - ) - - # JSON type for a single dimension's rectilinear spec: # bare int (uniform shorthand), or list of ints / [value, count] RLE pairs. RectilinearDimSpecJSON = int | list[int | list[int]] @@ -696,8 +679,10 @@ def from_dict(cls, data: dict[str, JSON]) -> Self: raise TypeError(f"Invalid fill_value: {fill!r}") from e # check if there are extra keys - allowed_extra_fields = extract_extra_fields( - _data, reserved_keys=ARRAY_METADATA_KEYS, node_type="array" + allowed_extra_fields = parse_extra_fields( + {k: v for k, v in _data.items() if k not in ARRAY_METADATA_KEYS}, + reserved_keys=ARRAY_METADATA_KEYS, + node_type="array", ) # TODO: replace this with a real type check! _data_typed = cast(ArrayMetadataJSON_V3, _data) diff --git a/src/zarr/core/sync_group.py b/src/zarr/core/sync_group.py index 8af514e938..6dcd5b3271 100644 --- a/src/zarr/core/sync_group.py +++ b/src/zarr/core/sync_group.py @@ -103,7 +103,7 @@ def create_hierarchy( nodes = {'a': GroupMetadata(attributes={'name': 'leaf'})} nodes_created = dict(create_hierarchy(store=store, nodes=nodes)) print(nodes) - # {'a': GroupMetadata(attributes={'name': 'leaf'}, zarr_format=3, consolidated_metadata=None, node_type='group')} + # {'a': GroupMetadata(attributes={'name': 'leaf'}, zarr_format=3, consolidated_metadata=None, node_type='group', extra_fields={})} ``` """ coro = create_hierarchy_async(store=store, nodes=nodes, overwrite=overwrite)