diff --git a/packages/zarr-metadata/changes/4574.bugfix.1.md b/packages/zarr-metadata/changes/4574.bugfix.1.md new file mode 100644 index 0000000000..cd474dc82b --- /dev/null +++ b/packages/zarr-metadata/changes/4574.bugfix.1.md @@ -0,0 +1,7 @@ +**Breaking:** A problem in a v2 struct dtype is located where the document + writes it. A record's problem was located under a `fields` key the + document does not have, at `("dtype", "fields", 0, 1)`; it is now at + `("dtype", 0, 1)`, and carries the value found there as its `input`. + `fields()` places each record's type there too. A definition's + functions still find the record types in `nested` at + `("fields", 0, 1)`, their place in the configuration. diff --git a/packages/zarr-metadata/changes/4574.bugfix.2.md b/packages/zarr-metadata/changes/4574.bugfix.2.md new file mode 100644 index 0000000000..4342fc151c --- /dev/null +++ b/packages/zarr-metadata/changes/4574.bugfix.2.md @@ -0,0 +1,4 @@ +A v2 dtype written as the string `"struct"` is refused as a string that is + not a NumPy typestr, like any other, instead of being read as a struct + with no records. `struct` is the name the definition of a records array + is filed under, not part of the v2 dtype grammar. diff --git a/packages/zarr-metadata/changes/4574.bugfix.3.md b/packages/zarr-metadata/changes/4574.bugfix.3.md new file mode 100644 index 0000000000..adec05b00c --- /dev/null +++ b/packages/zarr-metadata/changes/4574.bugfix.3.md @@ -0,0 +1,5 @@ +A group listed in v3 consolidated metadata whose own document has a problem, + such as attributes that are not an object, is reported for that problem + alone. It was also reported for disagreeing with the outer listing's copy + of the same node, a difference that came only from the problem. A copy + whose only problem is an unknown key is still compared. diff --git a/packages/zarr-metadata/changes/4574.bugfix.4.md b/packages/zarr-metadata/changes/4574.bugfix.4.md new file mode 100644 index 0000000000..c9dffad04b --- /dev/null +++ b/packages/zarr-metadata/changes/4574.bugfix.4.md @@ -0,0 +1,7 @@ +A v2 struct record of the wrong length is reported as "expected an array of + 2 or 3 elements", where the message named only the three-element form. A + two-element record whose name is not a string, `[5, " str: + """Join `faults`, each a reason, into one clause: `a`, `a and b`, `a, b and c`.""" + if len(faults) <= 1: + return "".join(faults) + return f"{', '.join(faults[:-1])} and {faults[-1]}" + + def tree_problems(nodes: Mapping[str, NodeType | None]) -> tuple[ValidationProblem, ...]: """The reasons `nodes` (the node type of each node, keyed by a valid node path) do not form a tree. Reports one problem per affected node, at that node's path. @@ -129,4 +136,4 @@ def _names(path: str) -> list[str]: return [] if path == "/" else path[1:].split("/") -__all__ = ["NodeType", "tree_problems"] +__all__ = ["NodeType", "said", "tree_problems"] diff --git a/packages/zarr-metadata/src/zarr_metadata/_json.py b/packages/zarr-metadata/src/zarr_metadata/_json.py index 8cc582c3f8..a33dd52be0 100644 --- a/packages/zarr-metadata/src/zarr_metadata/_json.py +++ b/packages/zarr-metadata/src/zarr_metadata/_json.py @@ -652,11 +652,22 @@ def json_type(value: object) -> str: def refused_kind(value: object, allowed: Sequence[object]) -> ProblemKind: - """The kind of problem for `value` outside a closed set: `invalid_type` when no member of the set has its JSON type, else `invalid_value`.""" - same_type = json_type(value) in {json_type(entry) for entry in allowed} + """The kind of problem for `value` outside a closed set: `invalid_type` when no member of the set has its type, else `invalid_value`. + + An integer is a type of its own here, as everywhere a reader expects + one: `2.0` for `zarr_format` is the wrong type, as `4.0` in `shape` is. + """ + same_type = _set_type(value) in {_set_type(entry) for entry in allowed} return "invalid_value" if same_type else "invalid_type" +def _set_type(value: object) -> str: + """The type `refused_kind` compares: the JSON type, with an integer told apart from any other number.""" + if isinstance(value, int) and not isinstance(value, bool): + return "an integer" + return json_type(value) + + def is_canonical_json(value: object, *, finite: bool = True) -> TypeGuard[JSONValue]: """Whether `value` already uses the concrete containers in `JSONValue`, nested no deeper than `JSON_DEPTH`. diff --git a/packages/zarr-metadata/src/zarr_metadata/_typed_json.py b/packages/zarr-metadata/src/zarr_metadata/_typed_json.py index 901d15a9b5..280bae918a 100644 --- a/packages/zarr-metadata/src/zarr_metadata/_typed_json.py +++ b/packages/zarr-metadata/src/zarr_metadata/_typed_json.py @@ -454,10 +454,19 @@ def _named(annotation: object, seen: frozenset[object]) -> tuple[str, str]: return choices(values), f"values in [{listed}]" if is_union(inner): # Name each shape once, since two TypedDicts are both "an object". - # A broader shape absorbs a narrower one: "a number" absorbs "an - # integer", and "a string" absorbs a `Literal` of strings. + # Arrays of two or more fixed lengths are named by their lengths: + # a union reports an array against itself only when it has none of + # them. A broader shape absorbs a narrower one: "a number" absorbs + # "an integer", and "a string" absorbs a `Literal` of strings. branches = get_args(inner) - named = dict.fromkeys(_named(branch, seen) for branch in branches) + lengths = sorted({length for length in map(fixed_length, branches) if length is not None}) + if len(lengths) > 1: + listed = f"{', '.join(map(str, lengths[:-1]))} or {lengths[-1]}" + arrays = (f"an array of {listed} elements", f"arrays of {listed} elements") + rest = [branch for branch in branches if fixed_length(branch) is None] + named = dict.fromkeys([arrays, *(_named(branch, seen) for branch in rest)]) + else: + named = dict.fromkeys(_named(branch, seen) for branch in branches) if ("a number", "numbers") in named: named.pop(("an integer", "integers"), None) if ("a string", "strings") in named: @@ -543,6 +552,22 @@ def shape_of(annotation: object) -> str | None: return None +def fixed_length(annotation: object) -> int | None: + """The length of an array of a fixed length, such as `tuple[str, int]`; None for any other annotation, including `tuple[int, ...]`.""" + inner = _unaliased(annotation) + if get_origin(inner) is not tuple: + return None + arguments = get_args(inner) + if len(arguments) == 2 and arguments[1] is Ellipsis: + return None + return len(arguments) + + +def _has_length(length: int | None, value: object) -> bool: + """Whether `value` has `length` elements, when a branch is an array of a fixed length. Any value passes a branch without one.""" + return length is None or (is_list_or_tuple(value) and len(value) == length) + + def has_shape(shape: str | None, value: object) -> bool: if shape is None: return True @@ -640,8 +665,8 @@ def parse(value: object, loc: Loc) -> Parsed: return parse -Branch: TypeAlias = tuple[str | None, Parser, frozenset[str] | None] -"""A branch of a union: its top-level shape (from `shape_of`), its parser, and its keys if it is a TypedDict.""" +Branch: TypeAlias = tuple[str | None, int | None, Parser, frozenset[str] | None] +"""A branch of a union: its top-level shape (from `shape_of`), its length if it is an array of a fixed length (from `fixed_length`), its parser, and its keys if it is a TypedDict.""" Tag: TypeAlias = tuple[str, Mapping[tuple[type, object], int]] """A key that every branch requires as a `Literal`, and the branch that each of its values selects.""" @@ -657,7 +682,8 @@ def any_of( """A member whose type is a union of shapes, parsed by the matching branch. The parser tries only the branches whose top-level shape matches the - value. This way a bad element inside an array is reported at its + value, and an array of a fixed length only on an array of that length. + This way a bad element inside an array is reported at its position inside the array, rather than the whole array being reported as wrong. A value that matches no branch's shape is reported once, with the expected shapes. When every branch is a TypedDict that @@ -683,8 +709,8 @@ def parse(value: object, loc: Loc) -> Parsed: return _by_tag(branches, tag, value, loc) clean: list[tuple[int, int, Parsed]] = [] failed: list[tuple[int, int, Parsed]] = [] - for index, (shape, branch, keys) in enumerate(branches): - if not has_shape(shape, value): + for index, (shape, length, branch, keys) in enumerate(branches): + if not has_shape(shape, value) or not _has_length(length, value): continue result = branch(value, loc) if len(result[1]) != 0: @@ -716,7 +742,8 @@ def _by_tag(branches: Sequence[Branch], tag: Tag, value: object, loc: Loc) -> Pa index = picks.get((type(said), said)) if _hashable(said) else None if index is None: return value, (outside_of((*loc, key), said, tuple(entry for _, entry in picks)),) - return branches[index][1](value, loc) + _, _, parse, _ = branches[index] + return parse(value, loc) def _keys_of(value: object) -> AbstractSet[str] | None: @@ -953,7 +980,7 @@ def _union(inner: object, leaf: Leaf, building: _Building) -> Parser | None: if isinstance(typeddict, type) and is_typeddict(typeddict) else None ) - branches.append((shape_of(branch), member, keys)) + branches.append((shape_of(branch), fixed_length(branch), member, keys)) literals = frozenset( index for index, branch in enumerate(arguments) if get_origin(_unaliased(branch)) is Literal ) diff --git a/packages/zarr-metadata/src/zarr_metadata/definition/_definition.py b/packages/zarr-metadata/src/zarr_metadata/definition/_definition.py index 3e7ec67ef9..e805d71f8e 100644 --- a/packages/zarr-metadata/src/zarr_metadata/definition/_definition.py +++ b/packages/zarr-metadata/src/zarr_metadata/definition/_definition.py @@ -61,6 +61,7 @@ from typing_extensions import TypeAliasType, TypedDict, TypeVar, is_typeddict from zarr_metadata._common import JSONValue, ZarrV3NamedConfigJSON +from zarr_metadata._hierarchy import said from zarr_metadata._json import ( Problems, ValidationProblem, @@ -254,9 +255,14 @@ def _refined( return refine_json(data, loc) @classmethod - def _configuration_loc(cls, loc: Loc) -> Loc: - """The location of the configuration of a field at `loc`: under `configuration` for v3, or at the field itself for a format that writes the parameters next to the name.""" - return (*loc, "configuration") + def _name_part(cls, key: str) -> str: + """How a message names the part of a name that encodes the configuration member `key`, for a kind whose names encode configuration: `key` itself by default.""" + return key + + @classmethod + def _in_configuration(cls, loc: Loc, place: Loc) -> Loc: + """The location in the document of `place`, a location in the configuration of a field at `loc`: under `configuration` for v3, or under the field itself for a format that writes the parameters next to the name. `place` is `()` for the configuration itself.""" + return (*loc, "configuration", *place) @classmethod def _name_loc(cls, loc: Loc) -> Loc: @@ -1209,15 +1215,15 @@ def fields_of( ) -> Iterator[tuple[Loc, ResolvedField[Any]]]: """Yield `resolved` (a field a scope read) with its location, then each nested field recursively, each with its location. - `loc` is the location of `resolved`. A nested field is located inside - the configuration, at the kind's `_configuration_loc` followed by the - field's place, the same way `resolve` locates problems. Each field's - nested fields are its `nested`. A field whose configuration was not - checked has none. + `loc` is the location of `resolved`. A nested field is located where + the document writes its place in the configuration, as the kind's + `_in_configuration` says, the same way `resolve` locates problems. + Each field's nested fields are its `nested`. A field whose + configuration was not checked has none. """ yield loc, resolved for place, inner in resolved.nested.items(): - yield from fields_of(inner, (*resolved.read_as._configuration_loc(loc), *place)) + yield from fields_of(inner, resolved.read_as._in_configuration(loc, place)) def with_problems( @@ -1484,7 +1490,7 @@ def _read( _, carried = kind._filed_under(name) if carried is not None: return _read_carried(data, name, kind, definition, given, carried, loc) - at = kind._configuration_loc(loc) + at = kind._in_configuration(loc, ()) if given is None and definition.requires_configuration: missing = problem(at, f"{name!r} requires a configuration", "missing_key") return RefusedField(json=data, name=name, read_as=kind, definition=definition), missing @@ -1498,14 +1504,16 @@ def _read( written: dict[Loc, JSONValue] = {} inside: list[ValidationProblem] = [] for field in nested: - inside.extend(_envelope(field)) - inner, found_inside = _read(field.json, field.kind, context, field.loc) - within[field.loc[len(at) :]] = inner + place = field.loc[len(at) :] + placed = dataclasses.replace(field, loc=kind._in_configuration(loc, place)) + inside.extend(_envelope(placed)) + inner, found_inside = _read(field.json, field.kind, context, placed.loc) + within[place] = inner # The input was refined JSON, so the result is JSON, or a field # refused for its contents, never for not being JSON. written[field.loc] = cast("JSONValue", document_json(inner)) inside.extend(found_inside) - inside.extend(_sized(field, inner)) + inside.extend(_sized(placed, inner)) # The rules may look up a nested field by its name, so they run only # when every nested field has a name. Any other problem with a nested # field belongs to that field and is reported at its location, as with @@ -1524,15 +1532,50 @@ def _read( own.extend( ruled(definition, lambda: definition.rules(read_only(configuration), within), at) ) - if configuration is None or not _usable(own): + placed = _in_document(kind, loc, given is not None, own) + if configuration is None or not _usable(placed): refused = RefusedField( json=data, name=name, read_as=kind, definition=definition, nested=within ) - return refused, (*own, *inside) + return refused, (*placed, *inside) read = AcceptedField( json=data, name=name, definition=definition, configuration=configuration, nested=within ) - return read, (*own, *inside) + return read, (*placed, *inside) + + +def in_document( + field: ResolvedField[Any], loc: Loc, problems: Sequence[ValidationProblem] +) -> Problems: + """`problems`, which a definition's functions found in the configuration of `field` (a field at `loc`), located under the configuration as the kind locates it, each moved to where the document writes it, as `resolve` places a definition's rules.""" + written = field.read_as._named_configuration(field.json)[1] is not None + return _in_document(field.read_as, loc, written, problems) + + +def _in_document( + kind: type[Definition[Any]], loc: Loc, written: bool, problems: Sequence[ValidationProblem] +) -> Problems: + """`problems`, found in the configuration of a field of `kind` at `loc` and located under the configuration's location, each moved to where the document writes its place, as `kind._in_configuration` says. + + `written` says whether the document writes a configuration. A field + whose configuration has a location of its own, and which writes none + (a v3 codec written as its name alone, or with no `configuration`), + has an empty configuration that the document does not hold. A member + found missing from it is then the configuration itself, missing, as + for a field that requires a configuration, and a problem with the + configuration as a whole is the field's. + """ + at = kind._in_configuration(loc, ()) + absent = not written and at != loc + placed: list[ValidationProblem] = [] + for found in problems: + place = found.loc[len(at) :] + if not absent: + there = kind._in_configuration(loc, place) + else: + there = loc if len(place) == 0 else at + placed.append(dataclasses.replace(found, loc=there)) + return tuple(placed) def _named(field: _NestedField) -> bool: @@ -1560,21 +1603,36 @@ def _read_carried( _, beside, _ = _checked( EmptyConfiguration, {} if given is None else given, - kind._configuration_loc(loc), + kind._in_configuration(loc, ()), ) - configuration, judged = definition.read_configuration(carried) + # A name is a single token, so a part the definition does not declare is + # a wrong value of the name, not an unknown key a reader may leave out. + # The parts it does not declare are reported in one problem, by how the + # name spells them, and the rest is read. + members = typeddict_keys(definition.configuration).members + declared = {key: value for key, value in carried.items() if key in members} + undeclared = dict.fromkeys(kind._name_part(key) for key in carried if key not in members) + configuration, judged = definition.read_configuration(declared) # A problem with the configuration encoded in the name belongs to the # field. It is located at the field, where the name is, and the # `input` and `ctx` that described a configuration member do not apply - # to the name, so they are cleared. + # to the name, so they are cleared. Each is a wrong value of the name: + # a part it lacks is not a missing key. problems = ( *beside, - *(dataclasses.replace(found, loc=loc, input=UNSET, ctx={}) for found in judged), + *( + dataclasses.replace(found, loc=loc, input=UNSET, ctx={}, kind="invalid_value") + for found in judged + ), ) - # A name is a single token. An undeclared part of the encoded - # configuration cannot be left out, as an extra key next to the name - # can, so any problem with the encoded configuration refuses the field. - if configuration is None or not _usable(beside) or len(judged) != 0: + if len(undeclared) != 0: + message = ( + f"expected a name {definition.name!r} reads, got {shown(name)}, which encodes " + f"{said(list(undeclared))} that it does not take" + ) + problems = (*problems, ValidationProblem(loc, message, "invalid_value")) + # Any problem with the encoded configuration refuses the field. + if configuration is None or not _usable(beside) or len(judged) != 0 or len(undeclared) != 0: refused = RefusedField(json=data, name=name, read_as=kind, definition=definition) return refused, problems return AcceptedField( @@ -1723,6 +1781,7 @@ def _replaced(value: JSONValue, path: Loc, new: JSONValue) -> JSONValue: "fill_value_problems", "format_of", "held", + "in_document", "is_field", "kind_of", "named_configuration", diff --git a/packages/zarr-metadata/src/zarr_metadata/model/_array_v2.py b/packages/zarr-metadata/src/zarr_metadata/model/_array_v2.py index ce749573ef..7768e68210 100644 --- a/packages/zarr-metadata/src/zarr_metadata/model/_array_v2.py +++ b/packages/zarr-metadata/src/zarr_metadata/model/_array_v2.py @@ -21,6 +21,7 @@ json_text, not_an_object, outside_of, + prefixed, refine_json, with_input, within, @@ -76,7 +77,7 @@ from zarr_metadata.v2.group import ZARR_V2_GROUP_METADATA_STORE_KEY if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterator, Sequence from zarr_metadata._typed_json import Loc from zarr_metadata.v2.array import ZarrV2ArrayMetadataStoreKey @@ -137,7 +138,9 @@ class ZarrV2ArrayMetadataReading: A field the document does not hold is `UNSET`, and so is a `filters` that is not an array, which holds no filters to read. A `compressor` - or `filters` written as `null` is None. + or `filters` written as `null` is None. Each problem and each field is + located in what was read: the document, or the files of a node that + `read_node_metadata_v2` reads, where `at` is `(".zarray",)`. """ dtype: ResolvedField[ZarrV2DataTypeDefinition[Any]] | UNSET = UNSET @@ -150,22 +153,29 @@ class ZarrV2ArrayMetadataReading: """Every reason the document is not valid.""" metadata: ZarrV2ArrayMetadata | None = None """The document's model, which holds these fields, when there is no problem; None otherwise.""" + at: Loc = () + """Where the document's fields are in what was read: `()` for a document read alone, `(".zarray",)` for the files of a node. A model's own reading is of its merged document, at `()`.""" def __reduce__(self) -> str | tuple[object, ...]: # Same as the v3 reading: pickle through the model, when there is one. if self.metadata is not None: - return (reading_of, (self.metadata,)) + return (_reading_at, (self.metadata, self.at)) return object.__reduce__(self) def fields(self) -> Iterator[tuple[Loc, ResolvedField[Any]]]: - """Each field in the document, as the scope read it, with its location: the dtype, followed by a struct's record types, then the compressor, then each filter at its index.""" + """Each field in the document, as the scope read it, with its location under `at`: the dtype, followed by a struct's record types, then the compressor, then each filter at its index.""" if self.dtype is not UNSET: - yield from fields_of(self.dtype, ("dtype",)) + yield from fields_of(self.dtype, (*self.at, "dtype")) if self.compressor is not UNSET and self.compressor is not None: - yield from fields_of(self.compressor, ("compressor",)) + yield from fields_of(self.compressor, (*self.at, "compressor")) if self.filters is not UNSET and self.filters is not None: for index, entry in enumerate(self.filters): - yield from fields_of(entry, ("filters", index)) + yield from fields_of(entry, (*self.at, "filters", index)) + + +def _reading_at(model: ZarrV2ArrayMetadata, at: Loc) -> ZarrV2ArrayMetadataReading: + """The reading that `model` holds, with its fields at `at`. Unpickling a reading that held a model rebuilds it with this function.""" + return dataclasses.replace(cast("ZarrV2ArrayMetadataReading", reading_of(model)), at=at) def read_array_v2( @@ -189,7 +199,8 @@ def read_array_v2( # implementations" (https://github.com/zarr-developers/zarr-specs/blob/fc7dd9c9beb5a50b87f9b08b00bf50fc0048482f/docs/v2/v2.0.rst#L91-L92), so a member outside # _ARRAY_METADATA_STANDARD_KEYS_V2 is not a problem just for being there. # Ignored does not mean unchecked: as in v3, the member must be JSON, - # and its key must be a string. + # and its key must be a string. Like a v3 member that nothing claims, + # it is not read, so it may hold any number, as attributes may. problems: list[ValidationProblem] = list(missing_keys(_ARRAY_METADATA_REQUIRED_KEYS_V2, doc)) extra_fields, found = other_members(doc, _ARRAY_METADATA_STANDARD_KEYS_V2) problems.extend(found) @@ -357,27 +368,63 @@ def node_document_v2( ) -> tuple[object, tuple[ValidationProblem, ...]]: """The merged v2 document of the node whose file is `key` in `files` (each file's decoded JSON, keyed by its name): the file's JSON, with the attributes at `.zattrs` when `files` holds it. - Returns the document and the problems found in merging. A missing - `key` gives None and that problem. A file that holds `attributes` - itself gives `beside_attributes`, since attributes belong in - `.zattrs`, and the document without that member, so that its reader - still reports every other problem. A file that is not an object is - returned as it is, for the document's reader to report. + `files` must hold `key`. Returns the document and the problems found + in merging, located in `files`. A file that holds `attributes` itself + gives `beside_attributes`, at the member in that file, since + attributes belong in `.zattrs`, and the document without that member, + so that its reader still reports every other problem. A file that is + not an object is returned as it is, for the document's reader to + report. """ - if key not in files: - return None, (ValidationProblem((key,), "missing store key", "missing_key"),) document = files[key] if not is_object(document): return document, () problems: tuple[ValidationProblem, ...] = () if "attributes" in document: - problems = with_input((beside_attributes,), document) + problems = prefixed(key, with_input((beside_attributes,), document)) document = {name: value for name, value in document.items() if name != "attributes"} if ZARR_V2_ATTRIBUTES_STORE_KEY not in files: return document, problems return {**document, "attributes": files[ZARR_V2_ATTRIBUTES_STORE_KEY]}, problems +def read_array_node_v2( + files: Mapping[str, object], context: ZarrV2Context +) -> ZarrV2ArrayMetadataReading: + """Read the v2 array node whose files are `files`, which hold its `.zarray`, as `read_node_metadata_v2` reads it: each problem and each field located in the file that holds it.""" + document, problems = node_document_v2( + files, ZARR_V2_ARRAY_METADATA_STORE_KEY, ARRAY_FILE_ATTRIBUTES + ) + reading = read_array_metadata_v2(document, context=context) + problems = (*problems, *in_files(reading.problems, ZARR_V2_ARRAY_METADATA_STORE_KEY, files)) + return dataclasses.replace( + reading, + problems=problems, + metadata=reading.metadata if len(problems) == 0 else None, + at=(ZARR_V2_ARRAY_METADATA_STORE_KEY,), + ) + + +def missing_store_key(key: str) -> MetadataValidationError: + """The error of a `from_key_value` whose mapping does not hold `key`, the node's own file.""" + return MetadataValidationError((ValidationProblem((key,), "missing store key", "missing_key"),)) + + +def in_files( + problems: Sequence[ValidationProblem], key: str, files: Mapping[str, object] +) -> tuple[ValidationProblem, ...]: + """`problems`, located in the merged document of a v2 node whose files are `files`, each located in the file that holds it: a problem in the attributes in `.zattrs`, when `files` holds one, and any other in `key`, the node's `.zarray` or `.zgroup`. The keys of `files` may be the entries of a `.zmetadata`, such as `a/.zarray`.""" + path, slash, _ = key.rpartition("/") + attributes_key = f"{path}{slash}{ZARR_V2_ATTRIBUTES_STORE_KEY}" + placed: list[ValidationProblem] = [] + for found in problems: + if attributes_key in files and found.loc[:1] == ("attributes",): + placed.append(dataclasses.replace(found, loc=(attributes_key, *found.loc[1:]))) + else: + placed.append(dataclasses.replace(found, loc=(key, *found.loc))) + return tuple(placed) + + ARRAY_FILE_ATTRIBUTES: Final = ValidationProblem( ("attributes",), f"unexpected key 'attributes' in {ZARR_V2_ARRAY_METADATA_STORE_KEY}: an array's attributes " @@ -616,7 +663,7 @@ def dimension_separator(self) -> ZarrV2ArrayDimensionSeparator: @property def unclaimed_fields(self) -> dict[Loc, UnclaimedField]: - """Every field nothing in scope claims, its dtype or a codec, located as `reading.fields()` locates it: at its place in the document, except that a struct dtype's field types are located by record and position, as their problems are. + """Every field nothing in scope claims, its dtype or a codec, located as `reading.fields()` locates it: at its place in the document, where a struct dtype's field types are at their record and position, as their problems are. This answers whether a reader can open the array: a reader cannot decode data with a dtype or a codec it does not understand, and the @@ -795,11 +842,9 @@ def from_key_value( ) if len(problems) != 0: raise MetadataValidationError(problems) - document, problems = node_document_v2( - files, ZARR_V2_ARRAY_METADATA_STORE_KEY, ARRAY_FILE_ATTRIBUTES - ) - if len(problems) != 0: - if document is not None: - problems = (*problems, *read_array_metadata_v2(document, context=context).problems) - raise MetadataValidationError(problems) - return cls(document, context=context) + if ZARR_V2_ARRAY_METADATA_STORE_KEY not in files: + raise missing_store_key(ZARR_V2_ARRAY_METADATA_STORE_KEY) + reading = read_array_node_v2(files, context) + if reading.metadata is None: + raise MetadataValidationError(reading.problems) + return reading.metadata diff --git a/packages/zarr-metadata/src/zarr_metadata/model/_group_v2.py b/packages/zarr-metadata/src/zarr_metadata/model/_group_v2.py index b26a46ca1b..3eaafec172 100644 --- a/packages/zarr-metadata/src/zarr_metadata/model/_group_v2.py +++ b/packages/zarr-metadata/src/zarr_metadata/model/_group_v2.py @@ -11,7 +11,7 @@ from typing_extensions import TypeAliasType, TypedDict, Unpack from zarr_metadata._common import JSONValue -from zarr_metadata._hierarchy import NodeType, tree_problems +from zarr_metadata._hierarchy import NodeType, said, tree_problems from zarr_metadata._json import ( MetadataValidationError, ValidationProblem, @@ -22,6 +22,7 @@ json_text, not_an_object, object_at, + prefixed, refine_json, refine_user_data, refined_object, @@ -36,8 +37,10 @@ ZarrV2ArrayMetadata, ZarrV2ArrayMetadataReading, decoded_node_files_v2, + in_files, + missing_store_key, node_document_v2, - read_array_metadata_v2, + read_array_node_v2, read_array_v2, ) from zarr_metadata.model._keyed import Keyed @@ -104,9 +107,9 @@ def _group_problems_v2(value: object) -> tuple[ValidationProblem, ...]: ("attributes",), f"unexpected key 'attributes' in {ZARR_V2_GROUP_METADATA_STORE_KEY}: a group's attributes " f"belong in {ZARR_V2_ATTRIBUTES_STORE_KEY}", - "unknown_key", + "invalid_value", ) -"""The problem of a `.zgroup` that holds `attributes`, which `.zgroup` does not declare: a group's attributes belong in `.zattrs`.""" +"""The problem of a `.zgroup` that holds `attributes`, which belong in `.zattrs`. The kind is the one `.zarray` gives the same member: the key is not unknown, but taken by the attributes, and a caller who tolerates an unknown key would drop the attributes the member holds.""" @dataclass(frozen=True, slots=True) @@ -137,7 +140,13 @@ def read_group_metadata_v2( problems = _group_problems_v2(value) if len(problems) != 0: return ZarrV2GroupMetadataReading(problems=problems) - return ZarrV2GroupMetadata(value, context=context).reading + # The document was checked above, so the model is built without + # checking it again, as for a group in a `.zmetadata`. + document = refined_object(value) + attributes: dict[str, JSONValue] | UNSET = ( + object_at(document, "attributes") if "attributes" in document else UNSET + ) + return ZarrV2GroupMetadata._of(document, context, attributes).reading # pyright: ignore[reportPrivateUsage] @dataclass(frozen=True, slots=True) @@ -180,7 +189,11 @@ def read_node_metadata_v2( both, give a `ZarrV2UnknownNodeMetadataReading` with the problem. A `.zarray` or `.zgroup` that writes `attributes` itself gives the reading of its node type with no model, and with that problem - followed by the problems of the rest of the file. This is the + followed by the problems of the rest of the file. Each problem is + located in the file that holds it, such as `(".zarray", "order")` or + `(".zattrs", "x")`, and so is each field of an array's reading, whose + `at` is `(".zarray",)`. The model reads its merged document, so its + own `reading` locates fields there, at `("dtype",)`. This is the v2 counterpart of `read_node_metadata_v3`, which reads one `zarr.json` and its `node_type`. """ @@ -195,23 +208,9 @@ def read_node_metadata_v2( ) return ZarrV2UnknownNodeMetadataReading((both,)) if is_array: - document, problems = node_document_v2( - files, ZARR_V2_ARRAY_METADATA_STORE_KEY, ARRAY_FILE_ATTRIBUTES - ) - reading = read_array_metadata_v2(document, context=context) - if len(problems) != 0: - return dataclasses.replace( - reading, problems=(*problems, *reading.problems), metadata=None - ) - return reading + return read_array_node_v2(files, context) if is_group: - document, problems = node_document_v2( - files, ZARR_V2_GROUP_METADATA_STORE_KEY, _GROUP_FILE_ATTRIBUTES - ) - reading = read_group_metadata_v2(document, context=context) - if len(problems) != 0: - return ZarrV2GroupMetadataReading(problems=(*problems, *reading.problems)) - return reading + return _read_group_node_v2(files, context) neither = ValidationProblem( (), f"expected a {ZARR_V2_ARRAY_METADATA_STORE_KEY} or a {ZARR_V2_GROUP_METADATA_STORE_KEY}", @@ -220,6 +219,20 @@ def read_node_metadata_v2( return ZarrV2UnknownNodeMetadataReading((neither,)) +def _read_group_node_v2( + files: Mapping[str, object], context: ZarrV2Context +) -> ZarrV2GroupMetadataReading: + """Read the v2 group node whose files are `files`, which hold its `.zgroup`, as `read_node_metadata_v2` reads it: each problem located in the file that holds it.""" + document, problems = node_document_v2( + files, ZARR_V2_GROUP_METADATA_STORE_KEY, _GROUP_FILE_ATTRIBUTES + ) + reading = read_group_metadata_v2(document, context=context) + problems = (*problems, *in_files(reading.problems, ZARR_V2_GROUP_METADATA_STORE_KEY, files)) + if len(problems) != 0: + return ZarrV2GroupMetadataReading(problems=problems) + return reading + + def node_metadata_from_key_value_v2( mapping: Mapping[StoreKey, bytes], *, context: ZarrV2Context | None = None ) -> ZarrV2NodeMetadata: @@ -413,14 +426,12 @@ def from_key_value( ) if len(problems) != 0: raise MetadataValidationError(problems) - document, problems = node_document_v2( - files, ZARR_V2_GROUP_METADATA_STORE_KEY, _GROUP_FILE_ATTRIBUTES - ) - if len(problems) != 0: - if document is not None: - problems = (*problems, *_group_problems_v2(document)) - raise MetadataValidationError(problems) - return cls(document, context=context) + if ZARR_V2_GROUP_METADATA_STORE_KEY not in files: + raise missing_store_key(ZARR_V2_GROUP_METADATA_STORE_KEY) + reading = _read_group_node_v2(files, context) + if reading.metadata is None: + raise MetadataValidationError(reading.problems) + return reading.metadata ZarrV2NodeMetadata = TypeAliasType("ZarrV2NodeMetadata", ZarrV2ArrayMetadata | ZarrV2GroupMetadata) @@ -659,39 +670,47 @@ def _v2_node_refines(node: ZarrV2NodeMetadata, other: ZarrV2NodeMetadata) -> boo def _entries_by_path( entries: Mapping[str, JSONValue], ) -> tuple[dict[str, dict[str, str]], list[ValidationProblem]]: - """The store key of each `.zarray`, `.zgroup` and `.zattrs` entry, by node path and then by file name. Also a problem for each extra key that names a file of a node already seen, since that entry would otherwise not be read.""" + """The store key of each `.zarray`, `.zgroup` and `.zattrs` entry, by node path and then by file name. Also a problem for each such key that does not name its file by the node's path, since a reader would not find that entry.""" by_path: dict[str, dict[str, str]] = {} problems: list[ValidationProblem] = [] for key in entries: path, _, name = key.rpartition("/") if name not in _NODE_FILES: continue - # A node's path, as a store names it: segments joined by a single `/`. - segments = [segment for segment in path.split("/") if segment != ""] - if any(segment in (".", "..") for segment in segments): - problems.append( - ValidationProblem( - ("metadata", key), - f"expected the path of a node, got {key!r}, which has a '.' or '..' segment", - "invalid_value", - ) - ) - continue - path = "/".join(segments) - files = by_path.setdefault(path, {}) - if name in files: + faults = _key_faults(key) + if len(faults) != 0: problems.append( ValidationProblem( ("metadata", key), - f"a second {name} for the node at {path!r}, which {files[name]!r} is", + f"expected the path of a node, got {key!r}, which {said(faults)}", "invalid_value", ) ) continue - files[name] = key + by_path.setdefault(path, {})[name] = key return by_path, problems +def _key_faults(key: str) -> list[str]: + """Each reason `key`, the key of a node's file in a `.zmetadata`, does not name the file by its node's path. + + A store names a node's file by the node's path: names joined by a + single `/`, with no leading `/`. zarr-python 2.x looks an entry up by + that key, so it never finds an entry written otherwise. Such a key is + refused, as a v3 listing refuses it. + """ + names = key.split("/")[:-1] + faults: list[str] = [] + if key.startswith("/"): + faults.append('starts with "/"') + names = names[1:] + if "" in names: + faults.append('holds an empty name between two "/"') + if any(name in (".", "..") for name in names): + faults.append("has a '.' or '..' segment") + return faults + + def _read_consolidated_v2( data: object, context: ZarrV2Context ) -> tuple[ @@ -702,10 +721,13 @@ def _read_consolidated_v2( ]: """Read `data`, a `.zmetadata` document. Return the document as read, with its keys in the order it writes them and each entry as read; the model of each node read in `context`, by path; the entry key of each node's files, by path and then by file name; and every problem, located in the document. - Each entry is read as the kind of document its key names. A `.zattrs` - entry is user data. Any other entry must be JSON by RFC 8259. A - `.zarray` or `.zgroup` entry is read as that document, merged with - its sibling `.zattrs`. Its problems are located under its own entry, + Each entry is read as the kind of document its key names. An entry + for a node's file is refined as user data, and its reader judges each + value the spec interprets: a `.zattrs` entry is user data, and a + `.zarray` or `.zgroup` reader refuses a non-finite number in a member + the spec defines, as it does in a store. Any other entry must be JSON + by RFC 8259. A `.zarray` or `.zgroup` entry is read as that document, + merged with its sibling `.zattrs`. Its problems are located under its own entry, and problems with the attributes under the `.zattrs` entry. The nodes and their files are empty when there is any problem. """ @@ -732,22 +754,18 @@ def _read_consolidated_v2( for key, value in entries.items(): if not isinstance(key, str): continue - refine = ( - refine_user_data - if key.rsplit("/", 1)[-1] == ZARR_V2_ATTRIBUTES_STORE_KEY - else refine_json - ) + refine = refine_user_data if key.rsplit("/", 1)[-1] in _NODE_FILES else refine_json entry, found = refine(value, ("metadata", key)) problems.extend(found) refined[key] = entry nodes: dict[str, ZarrV2NodeMetadata] = {} by_path: dict[str, dict[str, str]] = {} if len(problems) == 0: - # A second key for one file is reported along with the node - # problems. Every node is still read, so a user sees all problems - # at once. - by_path, doubled = _entries_by_path(refined) - problems.extend(doubled) + # A key that does not name its node's path is reported along with + # the node problems. Every other node is still read, so a user sees + # all problems at once. + by_path, misnamed = _entries_by_path(refined) + problems.extend(misnamed) for path, names in by_path.items(): node, found = _read_node_v2(path, names, refined, context) problems.extend(found) @@ -828,25 +846,14 @@ def _read_node_v2( if zattrs is not None: merged["attributes"] = entries[zattrs] - def located(found: tuple[ValidationProblem, ...]) -> list[ValidationProblem]: - placed: list[ValidationProblem] = [] - for problem in found: - if zattrs is not None and problem.loc[:1] == ("attributes",): - placed.append( - dataclasses.replace(problem, loc=("metadata", zattrs, *problem.loc[1:])) - ) - else: - placed.append(dataclasses.replace(problem, loc=("metadata", key, *problem.loc))) - return placed - if zarray is not None: reading, members = read_array_v2(merged, context) if members is None or len(beside) != 0: - return None, [*beside, *located(reading.problems)] + return None, [*beside, *prefixed("metadata", in_files(reading.problems, key, entries))] return ZarrV2ArrayMetadata._of(members.document, context, reading, members), [] # pyright: ignore[reportPrivateUsage] found = _group_problems_v2(merged) if len(found) != 0 or len(beside) != 0: - return None, [*beside, *located(found)] + return None, [*beside, *prefixed("metadata", in_files(found, key, entries))] document = refined_object(merged) attributes: dict[str, JSONValue] | UNSET = ( object_at(document, "attributes") if "attributes" in document else UNSET diff --git a/packages/zarr-metadata/src/zarr_metadata/model/_group_v3.py b/packages/zarr-metadata/src/zarr_metadata/model/_group_v3.py index daae22cdfe..838cdb3e22 100644 --- a/packages/zarr-metadata/src/zarr_metadata/model/_group_v3.py +++ b/packages/zarr-metadata/src/zarr_metadata/model/_group_v3.py @@ -1179,9 +1179,16 @@ def _nested_listing_problems( continue own = listing.members.get(path) flat_members = members.get(joined) + # A document with a problem of its own may still have members (a + # group whose attributes are not an object has them, without the + # attributes), so the problems decide whether the two are compared. + # An unknown key leaves the rest of the document read, so a copy + # with only that is still compared. if ( own is not None and flat_members is not None + and _read_whole(entry) + and _read_whole(readings[joined]) and not _read_alike(readings[joined], flat_members, entry, own) ): message = ( @@ -1192,6 +1199,11 @@ def _nested_listing_problems( return problems +def _read_whole(reading: ZarrV3NodeMetadataReading) -> bool: + """Whether every member of the document `reading` read was read: it has no problem but unknown keys, which are reported and left out.""" + return all(found.kind == "unknown_key" for found in reading.problems) + + def _read_alike( reading: ZarrV3NodeMetadataReading, members: ArrayMembersV3 | GroupMembersV3, diff --git a/packages/zarr-metadata/src/zarr_metadata/model/_shared.py b/packages/zarr-metadata/src/zarr_metadata/model/_shared.py index 4a03d906e9..a1f3d5c5e9 100644 --- a/packages/zarr-metadata/src/zarr_metadata/model/_shared.py +++ b/packages/zarr-metadata/src/zarr_metadata/model/_shared.py @@ -75,7 +75,12 @@ def other_members( additional_reserved_keys: frozenset[str] = frozenset(), at: Loc = (), ) -> tuple[dict[str, JSONValue], tuple[ValidationProblem, ...]]: - """Each member outside `standard_keys`, refined to JSON, and every problem found in them. A member that is not JSON is left out of the result.""" + """Each member outside `standard_keys`, refined as user data, and every problem found in them. A member that is not JSON is left out of the result. + + The spec does not interpret such a member, so like attributes, and + like a v3 member that nothing claims, it may hold `NaN`, `Infinity` + and `-Infinity`. + """ members: dict[str, JSONValue] = {} problems: list[ValidationProblem] = [] reserved_keys = standard_keys | additional_reserved_keys @@ -87,7 +92,7 @@ def other_members( continue if key in reserved_keys: continue - refined, found = refine_json(value, (*at, key)) + refined, found = refine_user_data(value, (*at, key)) problems.extend(within(found, at)) if len(found) == 0: members[key] = refined diff --git a/packages/zarr-metadata/src/zarr_metadata/v2/_definition.py b/packages/zarr-metadata/src/zarr_metadata/v2/_definition.py index 3e3e5ac1b9..4297d5fb3d 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v2/_definition.py +++ b/packages/zarr-metadata/src/zarr_metadata/v2/_definition.py @@ -105,10 +105,17 @@ def item_bytes(name: str) -> int | None: return size * CODE_POINT_BYTES if code == "U" else size +_TYPESTR_PARTS: Final[Mapping[str, str]] = { + "byteorder": "a byte order", + "itemsize": "a size", + "unit": "a unit", + "scale_factor": "a unit", +} +"""How a message names the part of a typestr that each member of its configuration comes from.""" + + def typestr_problem(name: str, at: Loc) -> ValidationProblem | None: - """Return the problem at `at` if `name` is not a typestr. Return None if it is a typestr or `struct`.""" - if name == STRUCT_NAME: - return None + """Return the problem at `at` if `name` is not a typestr. Return None if it is a typestr.""" if TYPESTR_PATTERN.fullmatch(name) is not None: if parse_typestr(name) is not None: return None @@ -134,9 +141,12 @@ class ZarrV2DataTypeDefinition(WithFillValue[C], kind=True, format=2): read for every typestr of the family. The byte order, size and unit of the typestr are its configuration. The rules state which of those the family accepts. `struct` is read for an array of field records, - with the configuration `{"fields": records}`. A problem in a record - is therefore located under `fields`, for example at - `("dtype", "fields", 0, 1)` for the type of the first record. + with the configuration `{"fields": records}`. The document writes the + records as the dtype itself, so a problem in a record is located + without `fields`, for example at `("dtype", 0, 1)` for the type of the + first record. A definition's functions still find the type of that + record in `nested` at `("fields", 0, 1)`, its place in the + configuration. """ label: ClassVar[str] = "v2 data type" @@ -153,6 +163,10 @@ def _refusal(self) -> str | None: @classmethod def _name_problem(cls, name: str, at: Loc) -> ValidationProblem | None: + # `struct` is the name a records array reads as, never a name a + # document writes: `_named_configuration` gives it only to an array. + if name == STRUCT_NAME: + return None return typestr_problem(name, at) @classmethod @@ -185,7 +199,8 @@ def _named_configuration( cls, value: object ) -> tuple[str | None, Mapping[str, object] | None, Problems]: if isinstance(value, str): - return value, None, () + # The string `struct` is not a typestr; only records read as a struct. + return (None, None, ()) if value == STRUCT_NAME else (value, None, ()) if isinstance(value, (list, tuple)): return STRUCT_NAME, {"fields": cast("JSONValue", value)}, () return None, None, () @@ -213,8 +228,16 @@ def _envelope_json(cls, name: str, configuration: Mapping[str, JSONValue]) -> JS return name @classmethod - def _configuration_loc(cls, loc: Loc) -> Loc: - return loc + def _name_part(cls, key: str) -> str: + # A unit carries its scale factor: `[10ms]` is one part of a typestr. + return _TYPESTR_PARTS[key] + + @classmethod + def _in_configuration(cls, loc: Loc, place: Loc) -> Loc: + # A v2 dtype has a configuration only as a struct's + # `{"fields": records}`, and the document writes the records as the + # dtype itself. + return (*loc, *place[1:]) @classmethod def _name_loc(cls, loc: Loc) -> Loc: @@ -280,8 +303,8 @@ def _envelope_json(cls, name: str, configuration: Mapping[str, JSONValue]) -> JS return {"id": name, **configuration} @classmethod - def _configuration_loc(cls, loc: Loc) -> Loc: - return loc + def _in_configuration(cls, loc: Loc, place: Loc) -> Loc: + return (*loc, *place) @classmethod def _name_loc(cls, loc: Loc) -> Loc: diff --git a/packages/zarr-metadata/src/zarr_metadata/v2/data_type/struct.py b/packages/zarr-metadata/src/zarr_metadata/v2/data_type/struct.py index 8640af285a..305c5a3460 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v2/data_type/struct.py +++ b/packages/zarr-metadata/src/zarr_metadata/v2/data_type/struct.py @@ -24,9 +24,6 @@ from zarr_metadata.definition._definition import Nested -# List the longer record first. A record that matches neither form is -# reported against the first branch, and a wrong shape is a deeper -# problem than a wrong length. ZarrV2StructRecord = ( tuple[str, ZarrV2DataTypeField, tuple[Annotated[int, Ge(0)], ...]] | tuple[str, ZarrV2DataTypeField] diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/_definition.py b/packages/zarr-metadata/src/zarr_metadata/v3/_definition.py index f67b861c9b..af28d02572 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/_definition.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/_definition.py @@ -500,8 +500,8 @@ def _unclaimed_json( return json @classmethod - def _configuration_loc(cls, loc: Loc) -> Loc: - return loc + def _in_configuration(cls, loc: Loc, place: Loc) -> Loc: + return (*loc, *place) @classmethod def _name_loc(cls, loc: Loc) -> Loc: diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/_hierarchy.py b/packages/zarr-metadata/src/zarr_metadata/v3/_hierarchy.py index ed32abc468..e5ea4e55c1 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/_hierarchy.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/_hierarchy.py @@ -19,11 +19,11 @@ from typing import TYPE_CHECKING, NewType, TypeGuard -from zarr_metadata._hierarchy import tree_problems +from zarr_metadata._hierarchy import said, tree_problems from zarr_metadata._json import MetadataValidationError, ValidationProblem, shown, with_input if TYPE_CHECKING: - from collections.abc import Mapping, Sequence + from collections.abc import Mapping from zarr_metadata._hierarchy import NodeType @@ -48,13 +48,6 @@ """ -def said(faults: Sequence[str]) -> str: - """Join `faults`, each a reason, into one clause: `a`, `a and b`, `a, b and c`.""" - if len(faults) <= 1: - return "".join(faults) - return f"{', '.join(faults[:-1])} and {faults[-1]}" - - def name_faults(name: str) -> list[str]: """The reasons `name` is not a node name, each as a message. Empty for a valid node name, including the root's `""`.""" faults: list[str] = [] diff --git a/packages/zarr-metadata/src/zarr_metadata/v3/_pipeline.py b/packages/zarr-metadata/src/zarr_metadata/v3/_pipeline.py index ae9e0ce325..2118d954d4 100644 --- a/packages/zarr-metadata/src/zarr_metadata/v3/_pipeline.py +++ b/packages/zarr-metadata/src/zarr_metadata/v3/_pipeline.py @@ -38,6 +38,7 @@ AcceptedField, ResolvedField, asked, + in_document, read_only, ruled, ) @@ -136,7 +137,8 @@ def read_pipeline( inner = _no_stages() if isinstance(codec, AcceptedField): configuration, nested = codec.configuration, codec.nested - problems.extend(_chunk_problems(definition, configuration, nested, incoming, at)) + found = _chunk_problems(definition, configuration, nested, incoming, at) + problems.extend(in_document(codec, (*loc, index), found)) inner, found = _inner_pipelines(definition, configuration, nested, incoming, at) problems.extend(found) stages.append(Stage(codec, incoming, inner)) diff --git a/packages/zarr-metadata/tests/model/test_array.py b/packages/zarr-metadata/tests/model/test_array.py index cd37495b27..ab5e7c046b 100644 --- a/packages/zarr-metadata/tests/model/test_array.py +++ b/packages/zarr-metadata/tests/model/test_array.py @@ -46,6 +46,7 @@ is_node_path_v3, read_array_metadata_v2, read_array_metadata_v3, + read_consolidated_metadata_v2, read_group_metadata_v2, read_group_metadata_v3, ) @@ -1111,7 +1112,7 @@ def test_v2_from_key_value_rejects_zarray_attributes() -> None: ZarrV2ArrayMetadata.from_key_value({".zarray": json.dumps(doc).encode()}) assert [(problem.loc, problem.kind) for problem in exc_info.value.problems] == [ - (("attributes",), "invalid_value") + ((".zarray", "attributes"), "invalid_value") ] @@ -1631,7 +1632,7 @@ def test_v2_structured_dtype_malformed_record_rejected() -> None: """A field record with the wrong arity is rejected, at the record.""" doc = dict(ZarrV2ArrayMetadata.create_default().to_json()) | {"dtype": (("a",),)} problems = read_array_metadata_v2(doc).problems - assert [p.loc for p in problems] == [("dtype", "fields", 0)] + assert [p.loc for p in problems] == [("dtype", 0)] def test_v2_order_literal_enforced() -> None: @@ -1727,9 +1728,9 @@ def test_array_zarr_format_rejects_float( document: object, read: Callable[[object], ZarrV2ArrayMetadataReading | ZarrV3ArrayMetadataReading], ) -> None: - """Integer-valued floats do not satisfy integer format literals.""" + """Integer-valued floats do not satisfy integer format literals. The float is the wrong type, as `4.0` is in `shape` and `1.0` in a codec's integer literal.""" problems = read(document).problems - assert [(p.loc, p.kind) for p in problems] == [(("zarr_format",), "invalid_value")] + assert [(p.loc, p.kind) for p in problems] == [(("zarr_format",), "invalid_type")] def test_array_v2_keeps_an_unknown_document_member() -> None: @@ -1751,9 +1752,8 @@ def test_array_v2_keeps_an_unknown_document_member() -> None: (object(), "invalid_type"), ({1, 2}, "invalid_type"), (b"\x00", "invalid_type"), - (math.nan, "invalid_value"), ], - ids=["object", "set", "bytes", "nan"], + ids=["object", "set", "bytes"], ) def test_error_array_v2_unknown_member_that_is_not_json(member: object, kind: str) -> None: """Ignored does not mean unchecked: an unknown member must be a JSON value, as in v3.""" @@ -1764,6 +1764,19 @@ def test_error_array_v2_unknown_member_that_is_not_json(member: object, kind: st ] +def test_an_unknown_member_may_hold_a_non_finite_number_in_either_format() -> None: + """A member the spec does not define is not read, so like attributes it may hold `NaN` and the infinities, in a v2 `.zarray` as in a v3 document, read alone or in consolidated metadata.""" + v2 = dict(ZarrV2ArrayMetadata.create_default().to_json()) | {"unexpected": math.nan} + v3 = dict(ZarrV3ArrayMetadata.create_default().to_json()) | {"unexpected": math.nan} + zmetadata = {"zarr_consolidated_format": 1, "metadata": {"a/.zarray": v2}} + assert read_array_metadata_v2(v2).problems == () + assert read_array_metadata_v3(v3).problems == () + assert read_consolidated_metadata_v2(zmetadata).problems == () + model = read_array_metadata_v2(v2).metadata + assert model is not None + assert math.isnan(cast("float", model.extra_fields["unexpected"])) + + @pytest.mark.parametrize("key", [7, None, True, (1, 2)], ids=["int", "none", "bool", "tuple"]) def test_error_array_v2_key_that_is_not_a_string(key: object) -> None: """A document's keys must be strings. Previously, the v2 parser returned a document like this one.""" @@ -1804,7 +1817,7 @@ def test_from_key_value_rejects_non_standard_json_constant() -> None: # Values of more than one byte require an `endian`. ( {"data_type": "int16", "codecs": ({"name": "bytes"},)}, - [(("codecs", 0, "configuration", "endian"), "missing_key")], + [(("codecs", 0, "configuration"), "missing_key")], ), ({"shape": (2,), "dimension_names": ("x", "y")}, [(("dimension_names",), "invalid_value")]), # The shape is not derived from the grid, and the scalar default @@ -2048,7 +2061,7 @@ def test_v2_from_key_value_scalar_root_raises_metadata_error() -> None: with pytest.raises(MetadataValidationError) as exc_info: ZarrV2ArrayMetadata.from_key_value({".zarray": b"null"}) assert [(problem.loc, problem.kind) for problem in exc_info.value.problems] == [ - ((), "invalid_type") + ((".zarray",), "invalid_type") ] diff --git a/packages/zarr-metadata/tests/model/test_construction.py b/packages/zarr-metadata/tests/model/test_construction.py index 037266de42..e3c15a6d2b 100644 --- a/packages/zarr-metadata/tests/model/test_construction.py +++ b/packages/zarr-metadata/tests/model/test_construction.py @@ -408,8 +408,8 @@ def test_error_consolidated_metadata_of_documents_at_bad_paths_is_refused() -> N (V2_GROUP, {"attributes": {1: "a"}}, [(("attributes",), "invalid_type")]), ( V2_CONSOLIDATED, - {"metadata": {"a/.zarray": {"x": math.nan}}}, - [(("metadata", "a/.zarray", "x"), "invalid_value")], + {"metadata": {"a/.zarray": {"x": object()}}}, + [(("metadata", "a/.zarray", "x"), "invalid_type")], ), ], ids=[ diff --git a/packages/zarr-metadata/tests/model/test_group.py b/packages/zarr-metadata/tests/model/test_group.py index 9f62600b74..4594ba1f4e 100644 --- a/packages/zarr-metadata/tests/model/test_group.py +++ b/packages/zarr-metadata/tests/model/test_group.py @@ -136,9 +136,9 @@ def test_group_zarr_format_rejects_float( document: object, read: Callable[[object], ZarrV2GroupMetadataReading | ZarrV3GroupMetadataReading], ) -> None: - """Integer-valued floats do not satisfy integer format literals.""" + """Integer-valued floats do not satisfy integer format literals. The float is the wrong type, as `4.0` is in `shape` and `1.0` in a codec's integer literal.""" problems = read(document).problems - assert [(p.loc, p.kind) for p in problems] == [(("zarr_format",), "invalid_value")] + assert [(p.loc, p.kind) for p in problems] == [(("zarr_format",), "invalid_type")] def _v2_consolidated_problems(document: object) -> list[tuple[tuple[str | int, ...], str]]: @@ -469,16 +469,18 @@ def test_group_v2_key_value_split() -> None: assert ZarrV2GroupMetadata.from_key_value(kv) == model -@pytest.mark.parametrize("extra_key", ["attributes", "vendor_extension"]) -def test_v2_group_from_key_value_rejects_zgroup_extra_members(extra_key: str) -> None: - """Raw `.zgroup` documents reject every non-spec member.""" +@pytest.mark.parametrize( + ("extra_key", "kind"), [("attributes", "invalid_value"), ("vendor_extension", "unknown_key")] +) +def test_v2_group_from_key_value_rejects_zgroup_extra_members(extra_key: str, kind: str) -> None: + """Raw `.zgroup` documents reject every non-spec member. `attributes` is not unknown, but belongs in `.zattrs`.""" doc: dict[str, object] = {"zarr_format": 2, extra_key: {}} with pytest.raises(MetadataValidationError) as exc_info: ZarrV2GroupMetadata.from_key_value({".zgroup": json.dumps(doc).encode()}) assert [(problem.loc, problem.kind) for problem in exc_info.value.problems] == [ - ((extra_key,), "unknown_key") + ((".zgroup", extra_key), kind) ] @@ -1155,7 +1157,7 @@ def test_group_v2_from_key_value_scalar_root_raises_metadata_error() -> None: with pytest.raises(MetadataValidationError) as exc_info: ZarrV2GroupMetadata.from_key_value({".zgroup": b"null"}) assert [(problem.loc, problem.kind) for problem in exc_info.value.problems] == [ - ((), "invalid_type") + ((".zgroup",), "invalid_type") ] @@ -1395,6 +1397,44 @@ def test_error_a_listed_group_s_own_listing_holds_the_documents_the_group_lists( assert [(p.loc, p.kind) for p in raised.value.problems] == [(at, "invalid_value")] +@pytest.mark.parametrize( + ("nested", "outer", "at"), + [ + ( + {"attributes": 5}, + {}, + ("metadata", "g", "consolidated_metadata", "metadata", "x", "attributes"), + ), + ({}, {"attributes": 5}, ("metadata", "g/x", "attributes")), + ], + ids=["nested-copy", "outer-copy"], +) +def test_error_a_listed_document_with_its_own_problem_is_reported_for_that_problem_alone( + nested: dict[str, Any], outer: dict[str, Any], at: tuple[str, ...] +) -> None: + """A listed group whose attributes are not an object is reported for its attributes, whichever copy of the node is bad, and is not also compared with the other copy, which it would read otherwise than.""" + document = _inline( + g=_group(consolidated_metadata=_inline(x=_group(**nested))), **{"g/x": _group(**outer)} + ) + found = read_consolidated_metadata_v3(document).problems + assert [(p.loc, p.kind) for p in found] == [(at, "invalid_type")] + + +def test_error_a_listed_document_with_only_an_unknown_key_is_still_compared() -> None: + """An unknown key leaves the rest of a listed group read, so a copy whose only problem is one is still compared with the other copy, and a difference in attributes is reported alongside it.""" + unknown = {**_inline(), "foo": 1} + nested = _group(attributes={"a": 1}, consolidated_metadata=unknown) + document = _inline( + g=_group(consolidated_metadata=_inline(x=nested)), **{"g/x": _group(attributes={"a": 2})} + ) + found = read_consolidated_metadata_v3(document).problems + x = ("metadata", "g", "consolidated_metadata", "metadata", "x") + assert [(p.loc, p.kind) for p in found] == [ + ((*x, "consolidated_metadata", "foo"), "unknown_key"), + (x, "invalid_value"), + ] + + def test_error_a_model_inside_consolidated_metadata_is_not_json() -> None: """A group holds its consolidated metadata as JSON, so a node model inside the member is a value that is not JSON. `ZarrV3ConsolidatedMetadata` is the place that takes node models as entries.""" member = _inline(a=ZarrV3ArrayMetadata.create_default()) diff --git a/packages/zarr-metadata/tests/model/test_pair_v2.py b/packages/zarr-metadata/tests/model/test_pair_v2.py index 0b55501409..400ecf9c6a 100644 --- a/packages/zarr-metadata/tests/model/test_pair_v2.py +++ b/packages/zarr-metadata/tests/model/test_pair_v2.py @@ -432,21 +432,39 @@ def test_error_a_node_entry_with_a_problem_is_refused_at_the_entry( assert raised.value.problems[0].loc == at -def test_error_two_entries_for_one_node_file_are_refused() -> None: - """Two keys that name the same file of the same node (`.zarray` and `/.zarray`) are a problem at the second key, since one of them would otherwise not be read. A conflict is located at the key as the document writes it.""" +@pytest.mark.parametrize( + ("key", "fault"), + [ + ("/.zarray", 'starts with "/"'), + ("/a/.zarray", 'starts with "/"'), + ("a//.zarray", 'holds an empty name between two "/"'), + ("a/./.zarray", "has a '.' or '..' segment"), + ], + ids=["root-with-slash", "leading-slash", "doubled-slash", "dot"], +) +def test_error_a_node_file_key_that_is_not_the_node_s_path_is_refused(key: str, fault: str) -> None: + """A key for a node's file names the file by the node's path, as a v3 listing names a node: no leading `/`, no empty name and no `.` or `..`. zarr-python looks an entry up by that key and would not find one written otherwise, so such a key is a problem at the key, and is not read as the node at another path.""" with pytest.raises(MetadataValidationError) as raised: ZarrV2ConsolidatedMetadata( - {"zarr_consolidated_format": 1, "metadata": {".zarray": ZARRAY, "/.zarray": ZARRAY}} + { + "zarr_consolidated_format": 1, + "metadata": {".zgroup": {"zarr_format": 2}, key: ZARRAY}, + } ) assert [(p.loc, p.kind) for p in raised.value.problems] == [ - (("metadata", "/.zarray"), "invalid_value") + (("metadata", key), "invalid_value") ] + assert fault in raised.value.problems[0].message + + +def test_error_a_conflict_is_located_at_the_key_the_document_writes() -> None: + """A conflict in a node's file is located at the entry's key in the `.zmetadata`.""" gained = ZarrV2ConsolidatedMetadata( - {"zarr_consolidated_format": 1, "metadata": {"/.zarray": ZARRAY}}, PRIVATE + {"zarr_consolidated_format": 1, "metadata": {"a/.zarray": ZARRAY}}, PRIVATE ) with pytest.raises(ScopeConflictError) as conflict: gained.refined_in(CORE_V2) - assert [c.loc for c in conflict.value.conflicts] == [("metadata", "/.zarray", "compressor")] + assert [c.loc for c in conflict.value.conflicts] == [("metadata", "a/.zarray", "compressor")] def test_the_node_type_is_exported() -> None: @@ -482,8 +500,8 @@ def test_each_node_alias_holds_its_union_at_run_time() -> None: ) -def test_error_a_second_key_for_a_node_file_hides_no_other_problem() -> None: - """A second key that names the same file of the same node is one problem among the document's problems. Every node is still read and each node's problems are reported, so a user sees everything at once. A leading or repeated `/` does not make a second node.""" +def test_error_a_misnamed_node_file_key_hides_no_other_problem() -> None: + """A key that does not name its node's path is one problem among the document's problems. Every other node is still read and each node's problems are reported, so a user sees everything at once.""" with pytest.raises(MetadataValidationError) as raised: ZarrV2ConsolidatedMetadata( { @@ -499,21 +517,6 @@ def test_error_a_second_key_for_a_node_file_hides_no_other_problem() -> None: ("metadata", "/a/.zarray"), ("metadata", "b/.zgroup", "zarr_format"), ] - one = ZarrV2ConsolidatedMetadata( - {"zarr_consolidated_format": 1, "metadata": {"/a/.zarray": ZARRAY}} - ) - assert set( - ZarrV2ConsolidatedMetadata( - { - "zarr_consolidated_format": 1, - "metadata": {"x/.zgroup": {"zarr_format": 2}, "x//y/.zarray": ZARRAY}, - } - ).metadata - ) == {"x", "x/y"} - assert set(one.metadata) == {"a"} - assert one == ZarrV2ConsolidatedMetadata( - {"zarr_consolidated_format": 1, "metadata": {"a/.zarray": ZARRAY}} - ) @pytest.mark.parametrize("name", ["__private", "zarr.json"]) @@ -529,7 +532,7 @@ def test_a_v2_node_may_have_a_name_that_only_v3_reserves(name: str) -> None: def test_error_a_key_with_a_dot_segment_is_no_node_s_path() -> None: - """A `.zmetadata` key whose path has a `.` or `..` segment names no node, unlike a key with a leading or repeated `/`. It is a problem at the key, and it does not make a second node next to the one it would name.""" + """A `.zmetadata` key whose path has a `.` or `..` segment names no node, as a key with a leading or repeated `/` names none. It is a problem at the key, and it does not make a second node next to the one it would name.""" with pytest.raises(MetadataValidationError) as raised: ZarrV2ConsolidatedMetadata( { diff --git a/packages/zarr-metadata/tests/model/test_problem_invariants.py b/packages/zarr-metadata/tests/model/test_problem_invariants.py new file mode 100644 index 0000000000..3d50c60ffd --- /dev/null +++ b/packages/zarr-metadata/tests/model/test_problem_invariants.py @@ -0,0 +1,428 @@ +"""Properties every reader's problems hold, over valid documents changed at random. + +Each test starts from valid documents of every kind a reader reads, and +changes them in a few places: a value replaced with any JSON value or with +a name a document might write, a member removed, a member added, a key +renamed. Whatever the reader makes of the result, its problems must be +located in the document it was given, and none may be reported twice. A +tool places a diagnostic at a problem's `loc`, so a location the document +does not have is a diagnostic nobody can place. + +The other tests pin one rule each, where an earlier release broke it: a +typestr is one token, so each of its problems is a wrong value of the +dtype, never an unknown or missing key; a float for an integer `Literal` +is the wrong type wherever the `Literal` sits; a member nothing claims +may hold any number in both formats; and both formats refuse the same +node paths. +""" + +from __future__ import annotations + +import copy +import math +from collections.abc import Callable, Mapping +from typing import Any, cast + +import pytest +from hypothesis import HealthCheck, event, given, settings +from hypothesis import strategies as st + +from zarr_metadata import UNSET +from zarr_metadata._json import ValidationProblem, value_at +from zarr_metadata.definition import RefusedField +from zarr_metadata.model import ( + read_array_metadata_v2, + read_array_metadata_v3, + read_consolidated_metadata_v2, + read_consolidated_metadata_v3, + read_group_metadata_v2, + read_group_metadata_v3, + read_node_metadata_v2, +) + +_PROPERTY = settings(max_examples=400, deadline=None, suppress_health_check=[HealthCheck.too_slow]) + +_V2_ARRAY: dict[str, Any] = { + "zarr_format": 2, + "shape": [4, 4], + "chunks": [2, 2], + "dtype": "|f4[]", max_size=6), + _NAMES, +) +_JSON = st.recursive( + _SCALARS, + lambda inner: st.one_of( + st.lists(inner, max_size=3), + st.dictionaries(st.one_of(_NAMES, st.text("ab", max_size=2)), inner, max_size=3), + ), + max_leaves=6, +) + + +def _places(value: object, at: tuple[str | int, ...] = ()) -> list[tuple[str | int, ...]]: + """Every location inside `value`, `value` itself first.""" + found = [at] + if isinstance(value, Mapping): + for key, item in value.items(): + found.extend(_places(item, (*at, key))) + elif isinstance(value, list): + for index, item in enumerate(value): + found.extend(_places(item, (*at, index))) + return found + + +def _container(document: object, at: tuple[str | int, ...]) -> dict[str, Any] | list[Any]: + """The object or array at `at` inside `document`, which a change edits in place.""" + found = cast("Any", document) + for part in at: + found = found[part] + return cast("dict[str, Any] | list[Any]", found) + + +_KEYS = st.one_of(_NAMES, st.text("ab/.", max_size=4)) + + +@st.composite +def _changed(draw: st.DrawFn, document: dict[str, Any]) -> dict[str, Any]: + """`document`, deep-copied and changed in one to three places.""" + changed = copy.deepcopy(document) + for _ in range(draw(st.integers(1, 3))): + places = [place for place in _places(changed) if len(place) != 0] + if len(places) == 0: + break + place = draw(st.sampled_from(places)) + parent = _container(changed, place[:-1]) + how = draw(st.sampled_from(["replace", "name", "remove", "add", "rename"])) + event(f"change: {how}") + value = draw(_NAMES if how == "name" else _JSON) + if isinstance(parent, dict): + key = cast("str", place[-1]) + if how in ("replace", "name"): + parent[key] = value + elif how == "remove": + del parent[key] + elif how == "add": + parent[draw(_KEYS)] = value + else: + renamed = draw(_KEYS) + if renamed not in parent: + parent[renamed] = parent.pop(key) + else: + index = cast("int", place[-1]) + if how in ("replace", "name"): + parent[index] = value + elif how == "remove": + del parent[index] + else: + parent.append(value) + return changed + + +def _misplaced( + document: object, problems: tuple[ValidationProblem, ...] +) -> list[ValidationProblem]: + """The problems whose location is not in `document`. A missing key is located where it would be, so its parent must be there.""" + misplaced: list[ValidationProblem] = [] + for found in problems: + at = found.loc[:-1] if found.kind == "missing_key" and len(found.loc) != 0 else found.loc + if value_at(document, at) is UNSET: + misplaced.append(found) + return misplaced + + +def _repeated(problems: tuple[ValidationProblem, ...]) -> list[tuple[object, ...]]: + seen: set[tuple[object, ...]] = set() + repeated: list[tuple[object, ...]] = [] + for found in problems: + key = (found.loc, found.kind, found.message) + if key in seen: + repeated.append(key) + seen.add(key) + return repeated + + +@pytest.mark.parametrize("which", list(_READERS)) +@_PROPERTY +@given(data=st.data()) +def test_every_problem_is_located_in_the_document_and_reported_once( + which: str, data: st.DataObject +) -> None: + """Whatever a reader makes of a changed document, each problem is located in that document, a missing key where it would go, and no problem is reported twice. An unknown key is located at the key itself.""" + read, valid = _READERS[which] + assert read(valid).problems == () + document = data.draw(_changed(valid)) + problems = read(document).problems + event("refused" if len(problems) != 0 else "read") + assert _misplaced(document, problems) == [] + assert _repeated(problems) == [] + for found in problems: + if found.kind == "unknown_key": + assert isinstance(found.loc[-1], str) + assert isinstance(value_at(document, found.loc[:-1]), Mapping) + + +@st.composite +def _node_files(draw: st.DrawFn) -> dict[str, object]: + """The files of a v2 array node, its `.zarray` changed and its `.zattrs` any JSON.""" + zarray = draw(_changed(_V2_ARRAY)) + return {".zarray": zarray, ".zattrs": draw(st.one_of(_JSON, st.just({"a": 1})))} + + +@_PROPERTY +@given(files=_node_files()) +def test_a_node_s_problems_are_located_in_the_files_it_reads(files: dict[str, object]) -> None: + """`read_node_metadata_v2` reads a mapping of files, so each problem is located in one of them.""" + problems = read_node_metadata_v2(files).problems + assert _misplaced(files, problems) == [] + + +_TYPESTRS = st.builds( + lambda order, code, size, unit: f"{order}{code}{size}{unit}", + st.sampled_from("<>|"), + st.sampled_from("bBiufcmMOSUVxe"), + st.sampled_from(["", "0", "1", "2", "3", "4", "8", "16"]), + st.sampled_from(["", "[ns]", "[10ms]", "[D]", "[x]", "[]", "[2]"]), +) + + +@_PROPERTY +@given(typestr=_TYPESTRS, nested=st.booleans()) +def test_a_typestr_s_problems_are_wrong_values_of_the_dtype(typestr: str, nested: bool) -> None: + """A typestr is one token. It reads, or each of its wrong parts is a wrong value of the dtype, at the dtype: no part is a key a tolerant reader could leave out, and none is missing.""" + dtype: object = [["a", typestr]] if nested else typestr + at: tuple[str | int, ...] = ("dtype", 0, 1) if nested else ("dtype",) + document = {**_V2_ARRAY, "dtype": dtype, "fill_value": None, "filters": None} + reading = read_array_metadata_v2(document) + found = {(problem.loc, problem.kind) for problem in reading.problems} + event("refused" if len(found) != 0 else "read") + assert found <= {(at, "invalid_value")} + # The typestr is refused exactly when it has a problem. A type code the + # spec does not list is unclaimed, which is no problem. + assert isinstance(dict(reading.fields())[at], RefusedField) == (len(found) != 0) + + +_INTEGER_LITERALS: dict[str, tuple[Reader, dict[str, Any], tuple[str, ...]]] = { + "v2-zarr_format": (read_array_metadata_v2, _V2_ARRAY, ("zarr_format",)), + "v3-zarr_format": (read_array_metadata_v3, _V3_ARRAY, ("zarr_format",)), + "v2-group-zarr_format": (read_group_metadata_v2, {"zarr_format": 2}, ("zarr_format",)), + "v2-consolidated-format": ( + read_consolidated_metadata_v2, + _V2_CONSOLIDATED, + ("zarr_consolidated_format",), + ), + "v2-blosc-shuffle": ( + read_array_metadata_v2, + { + **_V2_ARRAY, + "compressor": { + "id": "blosc", + "cname": "lz4", + "clevel": 5, + "shuffle": 1, + "blocksize": 0, + }, + }, + ("compressor", "shuffle"), + ), +} + + +@pytest.mark.parametrize("which", list(_INTEGER_LITERALS)) +@_PROPERTY +@given(value=st.floats(allow_nan=False, allow_infinity=False)) +def test_a_float_for_an_integer_literal_is_the_wrong_type(which: str, value: float) -> None: + """A float where a document requires one of a set of integers is the wrong type, whether or not it equals one of them, and wherever the set is declared.""" + read, valid, at = _INTEGER_LITERALS[which] + document = copy.deepcopy(valid) + cast("dict[str, Any]", _container(document, at[:-1]))[at[-1]] = value + event("integral" if value.is_integer() else "fractional") + assert [(p.loc, p.kind) for p in read(document).problems] == [(at, "invalid_type")] + + +_ANY_NUMBER = st.recursive( + st.one_of( + st.sampled_from([math.nan, math.inf, -math.inf]), + st.floats(), + st.integers(), + st.text(max_size=3), + st.none(), + ), + lambda inner: st.one_of( + st.lists(inner, max_size=3), st.dictionaries(st.text(max_size=3), inner, max_size=3) + ), + max_leaves=5, +) + + +@_PROPERTY +@given(member=_ANY_NUMBER) +def test_a_member_nothing_claims_may_hold_any_number_in_either_format(member: object) -> None: + """A member the spec does not define is not read, so like attributes it may hold `NaN` and the infinities, in a v2 `.zarray`, in a `.zmetadata` entry, and in a v3 document alike.""" + v2 = {**_V2_ARRAY, "unexpected": member} + v3 = {**_V3_ARRAY, "unexpected": {"must_understand": False, "value": member}} + event( + "non-finite" + if any(isinstance(x, float) and not math.isfinite(x) for x in _leaves(member)) + else "finite" + ) + assert read_array_metadata_v2(v2).problems == () + assert ( + read_consolidated_metadata_v2( + {"zarr_consolidated_format": 1, "metadata": {".zarray": v2}} + ).problems + == () + ) + assert read_array_metadata_v3(v3).problems == () + + +def _leaves(value: object) -> list[object]: + """Each value inside `value` that is not an object or an array.""" + inside = (value_at(value, place) for place in _places(value)) + return [leaf for leaf in inside if not isinstance(leaf, (Mapping, list))] + + +_PATHS = st.builds( + lambda slash, names: ("/" if slash else "") + "/".join(names), + st.booleans(), + st.lists(st.sampled_from(["a", "b", "", ".", ".."]), min_size=1, max_size=3), +) + + +@_PROPERTY +@given(path=_PATHS) +def test_both_formats_refuse_the_same_node_paths(path: str) -> None: + """A node path is names joined by single `/` characters, with no leading `/`, no empty name and no `.` or `..`, in a `.zmetadata` key as in a v3 listing. Each listed node has its parent groups listed too, so only the path decides.""" + names = path.split("/") + parents = ["/".join(names[:end]) for end in range(1, len(names))] + v2 = { + "zarr_consolidated_format": 1, + "metadata": { + ".zgroup": {"zarr_format": 2}, + **{f"{parent}/.zgroup": {"zarr_format": 2} for parent in parents}, + f"{path}/.zgroup": {"zarr_format": 2}, + }, + } + v3 = {**_INLINE, "metadata": {**dict.fromkeys(parents, _V3_GROUP), path: _V3_GROUP}} + refused_v2 = ("metadata", f"{path}/.zgroup") in { + p.loc for p in read_consolidated_metadata_v2(v2).problems + } + refused_v3 = ("metadata", path) in {p.loc for p in read_consolidated_metadata_v3(v3).problems} + event("refused" if refused_v3 else "read") + assert refused_v2 == refused_v3 diff --git a/packages/zarr-metadata/tests/model/test_read_array_metadata_v2.py b/packages/zarr-metadata/tests/model/test_read_array_metadata_v2.py index f5174b46de..afbd7fe0da 100644 --- a/packages/zarr-metadata/tests/model/test_read_array_metadata_v2.py +++ b/packages/zarr-metadata/tests/model/test_read_array_metadata_v2.py @@ -62,7 +62,7 @@ AcceptedField, None, (), - [("dtype",), ("dtype", "fields", 0, 1)], + [("dtype",), ("dtype", 0, 1)], ), ], ids=["default", "all-read", "unclaimed", "private-scope", "struct"], diff --git a/packages/zarr-metadata/tests/model/test_read_node_metadata_v2.py b/packages/zarr-metadata/tests/model/test_read_node_metadata_v2.py index 1b15c93f0b..f183b659c8 100644 --- a/packages/zarr-metadata/tests/model/test_read_node_metadata_v2.py +++ b/packages/zarr-metadata/tests/model/test_read_node_metadata_v2.py @@ -3,10 +3,12 @@ from __future__ import annotations import json +import pickle from typing import Any, cast import pytest +from zarr_metadata.definition import with_problems from zarr_metadata.model import ( ZarrV2ArrayMetadata, ZarrV2ArrayMetadataReading, @@ -120,14 +122,14 @@ def test_error_files_with_both_node_files_are_no_node() -> None: @pytest.mark.parametrize( - ("name", "document", "kind"), - [(".zarray", ZARRAY, "invalid_value"), (".zgroup", {"zarr_format": 2}, "unknown_key")], + ("name", "document"), + [(".zarray", ZARRAY), (".zgroup", {"zarr_format": 2})], ids=["array", "group"], ) def test_error_a_node_file_that_holds_attributes_is_refused_alike_in_a_store_and_a_zmetadata( - name: str, document: dict[str, Any], kind: str + name: str, document: dict[str, Any] ) -> None: - """A `.zarray` or `.zgroup` that holds `attributes` itself is refused, with a message that says the attributes belong in `.zattrs`. `read_node_metadata_v2` and `read_consolidated_metadata_v2` report the same problem, of the same kind, at the file and at its entry. For a `.zgroup`, which declares no other member, the kind is `unknown_key`.""" + """A `.zarray` or `.zgroup` that holds `attributes` itself is refused, with a message that says the attributes belong in `.zattrs`. `read_node_metadata_v2` and `read_consolidated_metadata_v2` report the same problem at the member in the file and in its entry. The kind is `invalid_value` for both files: the member is taken by the attributes, not unknown, and a caller who tolerates an unknown key would lose the attributes it holds.""" held = {**document, "attributes": {}} from_files = read_node_metadata_v2({name: held}).problems from_entries = read_consolidated_metadata_v2( @@ -136,9 +138,9 @@ def test_error_a_node_file_that_holds_attributes_is_refused_alike_in_a_store_and "metadata": {".zgroup": {"zarr_format": 2}, f"a/{name}": held}, } ).problems - assert [(p.loc, p.kind) for p in from_files] == [(("attributes",), kind)] + assert [(p.loc, p.kind) for p in from_files] == [((name, "attributes"), "invalid_value")] assert [(p.loc, p.kind) for p in from_entries] == [ - (("metadata", f"a/{name}", "attributes"), kind) + (("metadata", f"a/{name}", "attributes"), "invalid_value") ] assert from_files[0].message == from_entries[0].message assert "belong in .zattrs" in from_files[0].message @@ -151,15 +153,18 @@ def test_error_a_node_file_that_holds_attributes_is_refused_alike_in_a_store_and ".zarray", {**ZARRAY, "dtype": "bogus", "order": "Q"}, [ - (("attributes",), "invalid_value"), - (("dtype",), "invalid_value"), - (("order",), "invalid_value"), + ((".zarray", "attributes"), "invalid_value"), + ((".zarray", "dtype"), "invalid_value"), + ((".zarray", "order"), "invalid_value"), ], ), ( ".zgroup", {"zarr_format": 3}, - [(("attributes",), "unknown_key"), (("zarr_format",), "invalid_value")], + [ + ((".zgroup", "attributes"), "invalid_value"), + ((".zgroup", "zarr_format"), "invalid_value"), + ], ), ], ids=["array", "group"], @@ -167,7 +172,7 @@ def test_error_a_node_file_that_holds_attributes_is_refused_alike_in_a_store_and def test_error_a_node_file_that_holds_attributes_still_reports_its_other_problems( name: str, document: dict[str, Any], found: list[tuple[tuple[str, ...], str]] ) -> None: - """A `.zarray` or `.zgroup` that holds `attributes` does not hide its other problems: the rest of the file is still read, and its problems follow the one about `attributes`. `read_node_metadata_v2`, `read_consolidated_metadata_v2` (at the entry) and the model's `from_key_value` report the same problems.""" + """A `.zarray` or `.zgroup` that holds `attributes` does not hide its other problems: the rest of the file is still read, and its problems follow the one about `attributes`. `read_node_metadata_v2`, `read_consolidated_metadata_v2` and the model's `from_key_value` report the same problems, each in the file that holds it, or in its entry.""" held = {**document, "attributes": {}} model = ZarrV2ArrayMetadata if name == ".zarray" else ZarrV2GroupMetadata from_entries = read_consolidated_metadata_v2( @@ -180,15 +185,15 @@ def test_error_a_node_file_that_holds_attributes_still_reports_its_other_problem model.from_key_value({name: json.dumps(held).encode()}) assert [(p.loc, p.kind) for p in read_node_metadata_v2({name: held}).problems] == found assert [(p.loc, p.kind) for p in from_entries] == [ - (("metadata", f"a/{name}", *loc), kind) for loc, kind in found + (("metadata", f"a/{loc[0]}", *loc[1:]), kind) for loc, kind in found ] assert [(p.loc, p.kind) for p in raised.value.problems] == found def test_error_a_node_file_that_holds_attributes_next_to_a_bad_zattrs_reports_both() -> None: - """A `.zarray` that holds `attributes`, next to a `.zattrs` that is not an object, has two problems, one in each file, and neither hides the other. `read_node_metadata_v2` and the model's `from_key_value` merge the files, so both are at `attributes`. `read_consolidated_metadata_v2` reads the entries apart, so it reports each at its own entry.""" + """A `.zarray` that holds `attributes`, next to a `.zattrs` that is not an object, has two problems, one in each file, and neither hides the other. `read_node_metadata_v2`, the model's `from_key_value` and `read_consolidated_metadata_v2` each report them in the file, or the entry, that holds each.""" held = {**ZARRAY, "attributes": {}} - found = [(("attributes",), "invalid_value"), (("attributes",), "invalid_type")] + found = [((".zarray", "attributes"), "invalid_value"), ((".zattrs",), "invalid_type")] assert [ (p.loc, p.kind) for p in read_node_metadata_v2(_files(_zarray=held, _zattrs=5)).problems ] == found @@ -214,12 +219,29 @@ def test_error_a_store_file_that_is_not_json_raises_when_building_a_node() -> No assert [(p.loc, p.kind) for p in raised.value.problems] == [((".zarray",), "invalid_json")] +def test_a_v2_node_reading_locates_its_fields_in_the_files_it_read() -> None: + """A node's reading is of its files, so each field is under `.zarray`, as each problem is, and `with_problems` pairs a field with the problems in it. The model reads the merged document, so its own reading locates the same field at `dtype`. A pickled node reading keeps where its fields are.""" + struct = {**ZARRAY, "dtype": [["a", " None: - """A node whose document has a problem gives the reading of its node type with the problems, as the model's reader reports them, and no model.""" + """A node whose document has a problem gives the reading of its node type with the problems, located in the file that holds each, and no model.""" files = _files(_zarray={**ZARRAY, "shape": [-1]}) reading = read_node_metadata_v2(files) assert type(reading) is ZarrV2ArrayMetadataReading - assert [(p.loc, p.kind) for p in reading.problems] == [(("shape",), "invalid_value")] + assert [(p.loc, p.kind) for p in reading.problems] == [((".zarray", "shape"), "invalid_value")] assert reading.metadata is None @@ -247,24 +269,22 @@ def test_read_consolidated_metadata_v2_reads_a_zmetadata() -> None: ("entries", "key"), [ ({".zgroup": {"zarr_format": 2}, "a/.zarray": ZARRAY}, "a/.zarray"), - ({".zgroup": {"zarr_format": 2}, "/a/.zarray": ZARRAY}, "/a/.zarray"), ( { ".zgroup": {"zarr_format": 2}, "a/.zgroup": {"zarr_format": 2}, - "a//b/.zarray": ZARRAY, + "a/b/.zarray": ZARRAY, }, - "a//b/.zarray", + "a/b/.zarray", ), ({".zarray": ZARRAY}, ".zarray"), - ({"/.zarray": ZARRAY}, "/.zarray"), ], - ids=["path", "leading-slash", "doubled-slash", "root", "root-with-slash"], + ids=["path", "nested-path", "root"], ) def test_consolidated_fields_are_located_at_the_keys_the_document_writes( entries: dict[str, Any], key: str ) -> None: - """`fields()` locates each node's fields under the key that the `.zmetadata` writes for the node's `.zarray`, however that key spells the node's path, so each location leads into the document.""" + """`fields()` locates each node's fields under the key that the `.zmetadata` writes for the node's `.zarray`, so each location leads into the document.""" reading = read_consolidated_metadata_v2({"zarr_consolidated_format": 1, "metadata": entries}) assert reading.problems == () assert [loc for loc, _ in reading.fields()] == [("metadata", key, "dtype")] diff --git a/packages/zarr-metadata/tests/model/test_repair.py b/packages/zarr-metadata/tests/model/test_repair.py index efd6804b0d..18d7639040 100644 --- a/packages/zarr-metadata/tests/model/test_repair.py +++ b/packages/zarr-metadata/tests/model/test_repair.py @@ -325,12 +325,13 @@ def test_read_repaired_v2_node_reads_what_the_strict_read_refuses( files: dict[str, object], model: object, repaired_at: list[tuple[str | int, ...]] ) -> None: """The strict `read_node_metadata_v2` refuses the `.zarray` that zarr-python 3.0.0 to 3.0.6 wrote for a non-finite float or complex fill value, at each non-finite number. `read_repaired_node_metadata_v2` reads the repaired files, with the same result as a strict read of them, and lists a repair at each place the strict read refused. A valid node reads the same either way.""" - assert [p.loc for p in read_node_metadata_v2(files).problems] == repaired_at + at = [(".zarray", *loc) for loc in repaired_at] + assert [p.loc for p in read_node_metadata_v2(files).problems] == at repaired = read_repaired_node_metadata_v2(files) assert repaired.reading.problems == () assert repaired.reading.metadata == model assert [(r.loc, r.kind) for r in repaired.repairs] == [ - ((".zarray", *loc), "non_finite_fill_value") for loc in repaired_at + (loc, "non_finite_fill_value") for loc in at ] @@ -339,7 +340,9 @@ def test_read_repaired_v2_node_reports_what_no_repair_applies_to() -> None: files = {".zarray": {**NAN_ZARRAY, "order": "X"}} repaired = read_repaired_node_metadata_v2(files) assert [r.kind for r in repaired.repairs] == ["non_finite_fill_value"] - assert [(p.loc, p.kind) for p in repaired.reading.problems] == [(("order",), "invalid_value")] + assert [(p.loc, p.kind) for p in repaired.reading.problems] == [ + ((".zarray", "order"), "invalid_value") + ] assert repaired.reading.metadata is None diff --git a/packages/zarr-metadata/tests/model/test_store_json.py b/packages/zarr-metadata/tests/model/test_store_json.py index 378bce8d32..411e215613 100644 --- a/packages/zarr-metadata/tests/model/test_store_json.py +++ b/packages/zarr-metadata/tests/model/test_store_json.py @@ -279,7 +279,7 @@ def test_a_sequence_is_written_as_an_array() -> None: "zarray": ( ZarrV2ArrayMetadata.from_key_value, _stored(".zarray", {**_V2_ARRAY, "fill_value": math.nan}), - ("fill_value",), + (".zarray", "fill_value"), ), # Each `.zmetadata` entry is validated as the document type its key names. "zmetadata-entry": ( @@ -288,7 +288,10 @@ def test_a_sequence_is_written_as_an_array() -> None: ".zmetadata", { "zarr_consolidated_format": 1, - "metadata": {"a/.zattrs": {"x": math.nan}, "a/.zarray": {"fill_value": math.nan}}, + "metadata": { + "a/.zattrs": {"x": math.nan}, + "a/.zarray": {**_V2_ARRAY, "fill_value": math.nan}, + }, }, ), ("metadata", "a/.zarray", "fill_value"), diff --git a/packages/zarr-metadata/tests/model/test_v2_scope_reads.py b/packages/zarr-metadata/tests/model/test_v2_scope_reads.py index dc783eaa7b..b647a161e8 100644 --- a/packages/zarr-metadata/tests/model/test_v2_scope_reads.py +++ b/packages/zarr-metadata/tests/model/test_v2_scope_reads.py @@ -53,7 +53,7 @@ def test_a_v2_document_the_scope_reads_has_no_problem(changes: dict[str, Any]) - [ ({"dtype": "float32"}, ("dtype",), "invalid_value"), ({"dtype": " None: + """A float is the wrong type for an integer `Literal`, as it is for an `int`, whether the `Literal` stands alone or in a union. Another integer is the wrong value.""" + assert _found(_read(annotation, value)) == [((), kind)] + + def test_a_shape_is_described_as_a_message_would_name_it() -> None: assert describe(Literal[0, "auto"]) == 'one of ["auto", 0]' assert describe(Literal["0.5"]) == '"0.5"' @@ -956,6 +985,10 @@ def test_a_shape_is_described_as_a_message_would_name_it() -> None: assert describe(tuple[tuple[str, ...], ...]) == "an array of arrays of strings" assert describe(tuple[Literal["C", "F"], ...]) == 'an array of values in ["C", "F"]' assert describe(int | tuple[int, ...]) == "an integer or an array of integers" + # Arrays of fixed lengths are named by their lengths. + assert describe(tuple[str, int, int] | tuple[str, int] | None) == ( + "an array of 2 or 3 elements or null" + ) # A broader shape includes a narrower one: "a string" covers the hex # string spelling of a float32 and the strings of its non-finite values. assert describe(Float32FillValue) == "a number or a string" diff --git a/packages/zarr-metadata/tests/v2/test_data_types.py b/packages/zarr-metadata/tests/v2/test_data_types.py index ea0569ffaa..29778ebfa0 100644 --- a/packages/zarr-metadata/tests/v2/test_data_types.py +++ b/packages/zarr-metadata/tests/v2/test_data_types.py @@ -6,6 +6,8 @@ import pytest +from zarr_metadata import UNSET +from zarr_metadata._json import value_at from zarr_metadata.definition import ( AcceptedField, RefusedField, @@ -108,12 +110,15 @@ def test_a_type_code_the_spec_does_not_list_is_unclaimed(value: str) -> None: (" None: "short-record", "long-record", "nested-record-type", + "record-name", + "struct-string", + "record-struct-string", ], ) def test_error_a_typestr_the_family_does_not_take_is_a_problem(value: object, at: Loc) -> None: - """A typestr with a size, byte order or unit that its family does not accept is refused, and the problem is located at the field. A struct's problems are located under `fields`, at the record and the position inside it. If a record's type is refused, the problem is reported there and the struct is still read.""" + """A typestr with a size, byte order or unit that its family does not accept is refused with one `invalid_value` problem, located at the field. A struct's problems are located at the record and the position inside it, where the document writes them. If a record's type is refused, the problem is reported there and the struct is still read.""" kind, problems = _read(value) - assert kind is RefusedField or "fields" in at + assert kind is RefusedField or len(at) > 1 assert next(loc for loc, _ in problems) == at + if isinstance(value, str): + # A typestr is one token: one wrong value, never a key a reader may skip. + assert problems == [(at, "invalid_value")] + # Each location is in the document, so a tool can point at it. + assert value_at({"dtype": value}, at) is not UNSET def test_error_a_str_typestr_without_a_byte_order_states_its_size_in_bytes() -> None: @@ -158,9 +171,9 @@ def test_error_a_str_typestr_without_a_byte_order_states_its_size_in_bytes() -> def test_error_a_time_typestr_without_a_unit_is_refused_with_an_example_of_its_own_family( value: str, example: str ) -> None: - """` None: } +def test_error_a_struct_record_s_problem_holds_the_value_the_document_writes_there() -> None: + """A problem in a record is located in the document, so it carries the value written there as its `input`.""" + _, problems = resolve([["x", "float32"]], ZarrV2DataTypeDefinition, CONTEXT, ("dtype",)) + assert [(p.loc, p.input) for p in problems] == [(("dtype", 0, 1), "float32")] + + def test_a_struct_record_type_sits_where_the_document_writes_it() -> None: - """`fields_of` places a record's type under the struct's own configuration location, `("dtype", "fields", 0, 1)`, not under a `configuration` key a v2 document does not have.""" + """`fields_of` places a record's type where the document writes it, `("dtype", 0, 1)`: under neither a `configuration` key nor a `fields` key, which a v2 document does not have.""" resolved, _ = resolve( [["a", " None: - """A dtype field is either a string, which is its name, or an array of field records, which is a `struct` whose configuration is the records. Any other value has no name.""" + """A dtype field is either a string, which is its name, or an array of field records, which is a `struct` whose configuration is the records. Any other value has no name, and so has the string `struct`: it is not a typestr, and only records read as a struct.""" assert ZarrV2DataTypeDefinition._named_configuration(value) == (name, configuration, ()) @@ -104,7 +105,7 @@ def test_a_v2_dtype_is_a_string_or_records( ("value", "problems"), [ (" None: - """The envelope of a dtype field is valid when it is a typestr or an array of records. A string that is neither is reported as an invalid value, and the message names the typestr form. Any other value is reported as an invalid type.""" + """The envelope of a dtype field is valid when it is a typestr or an array of records. A string that is neither, `struct` included, is reported as an invalid value, and the message names the typestr form. Any other value is reported as an invalid type.""" found = ZarrV2DataTypeDefinition._envelope_problems(value) assert [(p.loc, p.kind) for p in found] == problems if value == "float32": @@ -122,11 +123,16 @@ def test_a_v2_dtype_envelope_is_a_typestr_or_records( def test_a_v2_dtype_writes_back_as_a_string_or_records() -> None: - """`envelope_json` writes a dtype as its string, and a struct as its records. Both the name and the configuration are located at the field.""" + """`envelope_json` writes a dtype as its string, and a struct as its records. The name is located at the field, and so is each record: a place under the struct's `fields` is located without `fields`.""" assert ZarrV2DataTypeDefinition._envelope_json(" None: """`envelope_json` writes a codec as `{"id": name, **parameters}`. The parameters are located at the field and the name is located under `id`.""" assert ZarrV2CodecDefinition._envelope_json("zlib", {"level": 1}) == {"id": "zlib", "level": 1} - assert ZarrV2CodecDefinition._configuration_loc(("compressor",)) == ("compressor",) + assert ZarrV2CodecDefinition._in_configuration(("compressor",), ("level",)) == ( + "compressor", + "level", + ) assert ZarrV2CodecDefinition._name_loc(("compressor",)) == ("compressor", "id") assert ZarrV2CodecDefinition._filed_under("zlib") == ("zlib", None) diff --git a/packages/zarr-metadata/tests/v3/test_pipelines.py b/packages/zarr-metadata/tests/v3/test_pipelines.py index 8361221e66..d844f8ab5c 100644 --- a/packages/zarr-metadata/tests/v3/test_pipelines.py +++ b/packages/zarr-metadata/tests/v3/test_pipelines.py @@ -277,14 +277,33 @@ def test_error_a_codec_with_a_problem_of_its_own_hands_on_a_chunk_nothing_is_kno def test_error_a_bytes_codec_without_endian_handed_values_of_several_bytes( data_type: JSONValue, ) -> None: - assert _problems(["bytes"], _of(data_type)) == [((0, "configuration", "endian"), "missing_key")] + assert _problems(["bytes"], _of(data_type)) == [((0, "configuration"), "missing_key")] + + +@pytest.mark.parametrize( + ("codec", "no_endian", "varying_size"), + [ + ("bytes", (0, "configuration"), (0,)), + ({"name": "bytes"}, (0, "configuration"), (0,)), + ( + {"name": "bytes", "configuration": {}}, + (0, "configuration", "endian"), + (0, "configuration"), + ), + ], + ids=["name", "object", "empty-configuration"], +) +def test_error_a_bytes_codec_s_problems_are_located_where_its_document_writes( + codec: JSONValue, no_endian: tuple[str | int, ...], varying_size: tuple[str | int, ...] +) -> None: + """A codec's chunk rules are located in its document. A missing `endian` is a member of the configuration the codec writes, or the configuration itself when it writes none, as for a codec that requires one. A problem with the configuration as a whole is the codec's when it writes none.""" + assert _problems([codec], _of("int16")) == [(no_endian, "missing_key")] + assert _problems([codec], _of("string")) == [(varying_size, "invalid_value")] def test_error_a_bytes_codec_without_endian_after_a_cast_past_a_codec_nothing_claims() -> None: # The cast declares the data type it outputs, whatever it receives. - assert _problems(["zfpy", _cast("float32"), "bytes"]) == [ - ((2, "configuration", "endian"), "missing_key") - ] + assert _problems(["zfpy", _cast("float32"), "bytes"]) == [((2, "configuration"), "missing_key")] @pytest.mark.parametrize("data_type", ["string", "bytes"]) @@ -470,7 +489,7 @@ def test_error_an_array_document_s_codecs_out_of_order_or_not_fitting_its_chunks assert [(p.loc, p.kind) for p in read_array_metadata_v3(document).problems] == [ (("codecs", 3), "invalid_value"), (("codecs", 0, "configuration", "order"), "invalid_value"), - (("codecs", 1, "configuration", "endian"), "missing_key"), + (("codecs", 1, "configuration"), "missing_key"), ] diff --git a/packages/zarr-metadata/tests/v3/test_rule_locations.py b/packages/zarr-metadata/tests/v3/test_rule_locations.py new file mode 100644 index 0000000000..585a70ae61 --- /dev/null +++ b/packages/zarr-metadata/tests/v3/test_rule_locations.py @@ -0,0 +1,93 @@ +"""Where an extension's rules are located: in the document as it is written, whatever the rules yield.""" + +from __future__ import annotations + +import dataclasses +from typing import TYPE_CHECKING + +import pytest + +from zarr_metadata import ValidationProblem +from zarr_metadata.definition import RefusedField, resolve +from zarr_metadata.v2.definition import CORE_V2, ZarrV2DataTypeDefinition +from zarr_metadata.v3.codec.bytes import BYTES_CODEC +from zarr_metadata.v3.data_type.raw import RAW_BYTES_DATA_TYPE +from zarr_metadata.v3.definition import CORE, CodecDefinition, DataTypeDefinition + +if TYPE_CHECKING: + from collections.abc import Iterator, Mapping + + from zarr_metadata.definition import Nested + from zarr_metadata.typed_json import Loc + + +@dataclasses.dataclass(frozen=True) +class _UnknownKeyAt: + """Rules that yield one `unknown_key` at `loc`, wherever an extension author puts it.""" + + loc: Loc + + def __call__( + self, configuration: Mapping[str, object], nested: Nested + ) -> Iterator[ValidationProblem]: + yield ValidationProblem(self.loc, "unexpected", "unknown_key") + + +def _wants_an_endian( + configuration: Mapping[str, object], nested: Nested +) -> Iterator[ValidationProblem]: + """Rules that always want an `endian`, and find a problem with the configuration as a whole.""" + if "endian" not in configuration: + yield ValidationProblem(("endian",), "expected an endian", "missing_key") + yield ValidationProblem((), "the whole configuration", "invalid_value") + + +@pytest.mark.parametrize("loc", [(), (0,), ("extra",)], ids=["root", "index", "key"]) +def test_error_a_rule_of_a_name_that_encodes_its_configuration_is_a_wrong_value_of_the_name( + loc: Loc, +) -> None: + """A name such as `r16` or ` None: + """A codec that writes no configuration has an empty one the document does not hold. A member its rules find missing is the configuration itself, missing, and a problem with the configuration as a whole is the codec's.""" + context = CORE.update( + codecs={**CORE.codecs, "bytes": dataclasses.replace(BYTES_CODEC, rules=_wants_an_endian)} + ) + _, problems = resolve(codec, CodecDefinition, context, ("codecs", 0)) + assert [(p.loc, p.kind) for p in problems] == [ + (no_endian, "missing_key"), + (whole, "invalid_value"), + ] diff --git a/packages/zarr-metadata/tests/v3/test_sharding.py b/packages/zarr-metadata/tests/v3/test_sharding.py index 3cb5982a56..c74b173c50 100644 --- a/packages/zarr-metadata/tests/v3/test_sharding.py +++ b/packages/zarr-metadata/tests/v3/test_sharding.py @@ -213,13 +213,13 @@ def test_error_an_inner_chunk_length_that_does_not_divide_the_shard( ), ( _shard([4, 4], codecs=["bytes"]), - [((0, "configuration", "codecs", 0, "configuration", "endian"), "missing_key")], + [((0, "configuration", "codecs", 0, "configuration"), "missing_key")], ), # The index has data type uint64, whose values take several bytes, and # has one more axis than the shard. ( _shard([4, 4], index=["bytes"]), - [((0, "configuration", "index_codecs", 0, "configuration", "endian"), "missing_key")], + [((0, "configuration", "index_codecs", 0, "configuration"), "missing_key")], ), ( _shard([4, 4], index=[_transpose(1, 0), LITTLE]), @@ -240,7 +240,7 @@ def test_error_an_array_document_s_shard_is_read_where_it_sits() -> None: assert [(p.loc, p.kind) for p in read_array_metadata_v3(document).problems] == [ (("codecs", 0, "configuration", "chunk_shape", 1), "invalid_value"), ( - ("codecs", 0, "configuration", "index_codecs", 0, "configuration", "endian"), + ("codecs", 0, "configuration", "index_codecs", 0, "configuration"), "missing_key", ), ]