From f390d89b068f06fb15300d00c7895f7bcafe8988 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 10:38:24 +0200 Subject: [PATCH 01/10] fix(py,ts): accept at 0.6 what the 0.6 schema declares The 0.6 axes schema requires only `name` and declares `longName`, and sets no additionalProperties. Both ports required `type`, modelled no `longName`, and handed the raw dict to a constructor, so a document our own schema calls valid died on `Axis.__init__()`. `type` and `longName` are now optional at 0.6, an axis is built from the fields it declares, and a key this version does not model is dropped with a warning naming it. `type` stays required at 0.4 and 0.5, where those schemas require it. An axis with no type carries no ordering constraint, in both ports. The omero channel list is positional, one entry per index of the `c` axis, so dropping a channel that lacks a `color` or a `window` renumbered every channel after it, silently, on the default read path. Both parsers now return one channel per entry with the fields it carries, `color` and `window` are optional, and `Omero.version` and `OmeroChannel.active` are read rather than lost on a round trip. A channel with no color passes the color-format rule: whether the field may be absent is the schema's decision. Ref #667. --- py/ngff_zarr/parse_metadata.py | 116 ++++++++++++------------- py/ngff_zarr/v04/zarr_metadata.py | 27 ++++-- py/ngff_zarr/v06/zarr_metadata.py | 31 ++++++- py/test/test_model_matches_schema.py | 105 ++++++++++++++++++++++ ts/src/types/zarr_metadata.ts | 10 ++- ts/src/utils/parse_metadata.ts | 94 ++++++++++---------- ts/src/utils/structural_validation.ts | 8 +- ts/test/compute_omero_test.ts | 60 ++++++------- ts/test/from_ngff_zarr_test.ts | 24 ++--- ts/test/model_matches_schema_test.ts | 72 +++++++++++++++ ts/test/omero_channel_chunking_test.ts | 2 +- ts/test/omero_test.ts | 76 ++++++++-------- 12 files changed, 425 insertions(+), 200 deletions(-) create mode 100644 py/test/test_model_matches_schema.py create mode 100644 ts/test/model_matches_schema_test.ts diff --git a/py/ngff_zarr/parse_metadata.py b/py/ngff_zarr/parse_metadata.py index ba043218..c97f1835 100644 --- a/py/ngff_zarr/parse_metadata.py +++ b/py/ngff_zarr/parse_metadata.py @@ -1,6 +1,6 @@ # SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC # SPDX-License-Identifier: MIT -from typing import Union +from typing import Any, Union from ._supported_versions import NgffVersion from .methods import Methods @@ -36,65 +36,65 @@ def _extract_method_metadata( return method, method_type, method_metadata -def _parse_omero(omero_data: Union[dict, None]) -> Omero | None: - """Parse OMERO metadata dictionary into Omero dataclass.""" - omero = None - if isinstance(omero_data, dict) and "channels" in omero_data: - channels_data = omero_data["channels"] - if isinstance(channels_data, list): - channels = [] - for channel in channels_data: - if not isinstance(channel, dict) or "window" not in channel: - continue - - window_data = channel["window"] - if not isinstance(window_data, dict): - continue - - # Handle backward compatibility for OMERO window metadata - # Some stores use min/max, others use start/end, some have both - if "start" in window_data and "end" in window_data: - # New format with start/end - start = float(window_data["start"]) # type: ignore - end = float(window_data["end"]) # type: ignore - if "min" in window_data and "max" in window_data: - # Both formats present - min_val = float(window_data["min"]) # type: ignore - max_val = float(window_data["max"]) # type: ignore - else: - # Only start/end, use them as min/max - min_val = start - max_val = end - elif "min" in window_data and "max" in window_data: - # Old format with min/max only - min_val = float(window_data["min"]) # type: ignore - max_val = float(window_data["max"]) # type: ignore - # Use min/max as start/end for backward compatibility - start = min_val - end = max_val - else: - # Invalid window data, skip this channel - continue - - channels.append( - OmeroChannel( - color=str(channel["color"]), # type: ignore - label=str(channel.get("label", None)) - if channel.get("label") is not None - else None, # type: ignore - window=OmeroWindow( - min=min_val, - max=max_val, - start=start, - end=end, - ), - ) - ) +def _parse_window(window_data: Any) -> OmeroWindow | None: + """Build an :class:`OmeroWindow` from whatever bounds a channel carries. + + Stores in the wild write ``min``/``max``, ``start``/``end``, or both. When + only one pair is present it stands for the other, which is how this library + has always read them. A window with neither pair keeps the bounds it has + and leaves the rest unset rather than making the channel unreadable. + """ + if not isinstance(window_data, dict): + return None + + def number(key): + value = window_data.get(key) + return None if value is None else float(value) + + minimum, maximum = number("min"), number("max") + start, end = number("start"), number("end") + if start is None and end is None: + start, end = minimum, maximum + elif minimum is None and maximum is None: + minimum, maximum = start, end + return OmeroWindow(min=minimum, max=maximum, start=start, end=end) - if channels: - omero = Omero(channels=channels) - return omero +def _parse_omero(omero_data: Union[dict, None]) -> Omero | None: + """Parse OMERO metadata into the :class:`Omero` dataclass. + + One channel comes back per entry. The list is positional, one entry per + index of the ``c`` axis (:func:`~ngff_zarr.compute_omero + .compute_omero_from_ngff_image` writes it that way), so dropping a channel + that lacks a ``color`` or a ``window`` would silently renumber every + channel after it. A field the entry does not carry is left unset, and + whether it may be absent is the schema's decision. + """ + if not isinstance(omero_data, dict) or "channels" not in omero_data: + return None + channels_data = omero_data["channels"] + if not isinstance(channels_data, list): + return None + + channels = [] + for channel in channels_data: + if not isinstance(channel, dict): + channels.append(OmeroChannel()) + continue + color = channel.get("color") + label = channel.get("label") + active = channel.get("active") + channels.append( + OmeroChannel( + color=None if color is None else str(color), + window=_parse_window(channel.get("window")), + label=None if label is None else str(label), + active=None if active is None else bool(active), + ) + ) + + version = omero_data.get("version") + return Omero(channels=channels, version=None if version is None else str(version)) def _parse_hcs_path(store_path: str) -> tuple[str, str | None]: diff --git a/py/ngff_zarr/v04/zarr_metadata.py b/py/ngff_zarr/v04/zarr_metadata.py index 22393d38..68c29389 100644 --- a/py/ngff_zarr/v04/zarr_metadata.py +++ b/py/ngff_zarr/v04/zarr_metadata.py @@ -230,19 +230,33 @@ class Dataset: @dataclass class OmeroWindow: - min: float - max: float - start: float - end: float + #: Every bound is optional, as in the OME-Zarr schema, which requires none + #: of them. A window that carries only some of its bounds is read with the + #: rest left unset rather than dropped. + min: float | None = None + max: float | None = None + start: float | None = None + end: float | None = None @dataclass class OmeroChannel: - color: str - window: OmeroWindow + #: ``color`` and ``window`` are optional so that a channel is read as + #: written. The list is positional, one entry per index of the ``c`` axis, + #: so a channel that cannot be fully built must still occupy its place. + color: str | None = None + window: OmeroWindow | None = None label: str | None = None + active: bool | None = None def validate_color(self): + """Raise when ``color`` is present and is not six hexadecimal digits. + + A channel with no ``color`` passes: whether the field may be absent is + the schema's decision, not this predicate's. + """ + if self.color is None: + return if not re.fullmatch(r"[0-9A-Fa-f]{6}", self.color): raise ValueError(f"Invalid color '{self.color}'. Must be 6 hex digits.") @@ -250,6 +264,7 @@ def validate_color(self): @dataclass class Omero: channels: list[OmeroChannel] + version: str | None = None @dataclass diff --git a/py/ngff_zarr/v06/zarr_metadata.py b/py/ngff_zarr/v06/zarr_metadata.py index f62cf4b3..c2c8c300 100644 --- a/py/ngff_zarr/v06/zarr_metadata.py +++ b/py/ngff_zarr/v06/zarr_metadata.py @@ -38,11 +38,38 @@ @dataclass class Axis: + #: The 0.6 axes schema requires only ``name`` and declares ``longName``, + #: so an axis carrying neither ``type`` nor ``longName`` is valid at this + #: version. ``type`` stays required at 0.4 and 0.5, where their schemas + #: require it. name: SupportedDims - type: AxesType + type: AxesType | None = None unit: Units | None = None orientation: AnatomicalOrientation | None = None discrete: bool | None = None + longName: str | None = None + + @classmethod + def from_dict(cls, axis: dict) -> "Axis": + """Build an axis from a metadata entry, keeping the fields it declares. + + The 0.6 axes schema sets no ``additionalProperties``, so a document may + carry a key this library does not model. Such a key is dropped with a + warning: the reader states what it ignored instead of failing on a + Python constructor, and a later spec release that adds a property is + read rather than refused. + """ + known = {field for field in cls.__dataclass_fields__} + unknown = sorted(set(axis) - known) + if unknown: + name = axis.get("name", "") + warnings.warn( + f"Axis '{name}' carries {unknown}, which this version of " + "ngff-zarr does not model; the field(s) are ignored.", + UserWarning, + stacklevel=2, + ) + return cls(**{key: value for key, value in axis.items() if key in known}) @dataclass @@ -795,7 +822,7 @@ def _from_zarr_attrs( coordinate_systems = [] for cs in root_attrs.get("coordinateSystems", []): - axes = [Axis(**axis) for axis in cs["axes"]] + axes = [Axis.from_dict(axis) for axis in cs["axes"]] coordinate_systems.append(CoordinateSystem(name=cs["name"], axes=axes)) diff --git a/py/test/test_model_matches_schema.py b/py/test/test_model_matches_schema.py new file mode 100644 index 00000000..82a22d6f --- /dev/null +++ b/py/test/test_model_matches_schema.py @@ -0,0 +1,105 @@ +# SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +# SPDX-License-Identifier: MIT +"""The dataclasses accept what the bundled schema of their version declares.""" + +import json +from pathlib import Path + +import dask.array as da +import numpy as np +import pytest +from ngff_zarr import NgffImage, from_ome_zarr, to_multiscales, to_ome_zarr, validate +from ngff_zarr.parse_metadata import _parse_omero +from ngff_zarr.v04.zarr_metadata import Omero, OmeroChannel, OmeroWindow + + +def _write_06(tmp_path): + image = NgffImage( + da.zeros((2, 8, 8), dtype=np.uint8, chunks=(2, 8, 8)), + ["c", "y", "x"], + {"c": 1.0, "y": 1.0, "x": 1.0}, + {"c": 0.0, "y": 0.0, "x": 0.0}, + ) + store = str(tmp_path / "s.ome.zarr") + to_ome_zarr( + store, to_multiscales(image, scale_factors=[], cache=False), version="0.6" + ) + return store + + +def _patch_first_axis(store, mutate): + doc_path = Path(store) / "zarr.json" + doc = json.loads(doc_path.read_text()) + systems = doc["attributes"]["ome"]["multiscales"][0]["coordinateSystems"] + mutate(systems[0]["axes"][0]) + doc_path.write_text(json.dumps(doc, indent=4)) + return doc["attributes"] + + +@pytest.mark.parametrize( + "mutate, description", + [ + (lambda axis: axis.update(longName="channel index"), "longName"), + (lambda axis: axis.pop("type", None), "no type"), + ], +) +def test_the_0_6_reader_accepts_what_its_schema_declares(tmp_path, mutate, description): + """The 0.6 axes schema requires only ``name`` and declares ``longName``.""" + store = _write_06(tmp_path) + attrs = _patch_first_axis(store, mutate) + + validate(attrs, version="0.6") + image = from_ome_zarr(store).images[0] + assert image.data.shape == (2, 8, 8), description + + +def test_an_unknown_axis_key_is_dropped_with_a_warning(tmp_path): + """The 0.6 axes schema sets no additionalProperties, so a later release may + add a property this version does not model.""" + store = _write_06(tmp_path) + _patch_first_axis(store, lambda axis: axis.update(madeUp=1)) + + with pytest.warns(UserWarning, match="madeUp"): + assert len(from_ome_zarr(store).images) == 1 + + +def test_every_omero_channel_keeps_its_place(): + """The channel list is positional, so dropping one renumbers the rest.""" + omero = _parse_omero( + { + "version": "0.4", + "channels": [ + { + "color": "FF0000", + "window": {"min": 0, "max": 255, "start": 0, "end": 255}, + "label": "first", + "active": True, + }, + {"color": "00FF00", "label": "no window"}, + {"window": {"min": 0, "max": 1}, "label": "no color"}, + ], + } + ) + assert [channel.label for channel in omero.channels] == [ + "first", + "no window", + "no color", + ] + assert omero.version == "0.4" + assert omero.channels[0].active is True + assert omero.channels[1].window is None + assert omero.channels[2].color is None + # min/max standing in for start/end, as this library has always read them + assert omero.channels[2].window == OmeroWindow(min=0.0, max=1.0, start=0.0, end=1.0) + + +def test_a_channel_without_a_color_passes_the_color_rule(): + """Whether the field may be absent is the schema's decision.""" + OmeroChannel().validate_color() + with pytest.raises(ValueError, match="6 hex digits"): + OmeroChannel(color="nope").validate_color() + + +def test_omero_defaults_are_writable(): + """An Omero built with no channel detail still serializes.""" + assert Omero(channels=[OmeroChannel()]).channels[0].window is None diff --git a/ts/src/types/zarr_metadata.ts b/ts/src/types/zarr_metadata.ts index 241ad572..94c6b4c9 100644 --- a/ts/src/types/zarr_metadata.ts +++ b/ts/src/types/zarr_metadata.ts @@ -28,6 +28,7 @@ export interface Axis { unit: AxisUnit | undefined; orientation?: AxisOrientation | AnatomicalOrientation | undefined; discrete?: boolean; + longName?: string; } /** @@ -210,8 +211,13 @@ export interface OmeroWindow { } export interface OmeroChannel { - color: string; - window: OmeroWindow; + /** + * `color` and `window` are optional so that a channel is read as written. + * The list is positional, one entry per index of the `c` axis, so a channel + * that cannot be fully built must still occupy its place. + */ + color?: string; + window?: OmeroWindow; label?: string; active?: boolean; } diff --git a/ts/src/utils/parse_metadata.ts b/ts/src/utils/parse_metadata.ts index 7fbd3cb5..9c1ef673 100644 --- a/ts/src/utils/parse_metadata.ts +++ b/ts/src/utils/parse_metadata.ts @@ -66,6 +66,43 @@ export function extractMethodMetadata( /** * Parse OMERO metadata dictionary into Omero interface. */ +/** + * Build an `OmeroWindow` from whatever bounds a channel carries. + * + * Stores in the wild write `min`/`max`, `start`/`end`, or both. When only one + * pair is present it stands for the other, which is how this library has + * always read them. A window with neither pair keeps the bounds it has and + * leaves the rest unset rather than making the channel unreadable. + */ +function parseOmeroWindow(windowData: unknown): OmeroWindow | undefined { + if (!windowData || typeof windowData !== "object") { + return undefined; + } + const data = windowData as Record; + const number = (key: string): number | undefined => { + const value = data[key]; + return value === undefined || value === null ? undefined : Number(value); + }; + + let min = number("min"); + let max = number("max"); + let start = number("start"); + let end = number("end"); + if (start === undefined && end === undefined) { + start = min; + end = max; + } else if (min === undefined && max === undefined) { + min = start; + max = end; + } + return { + ...(min !== undefined ? { min } : {}), + ...(max !== undefined ? { max } : {}), + ...(start !== undefined ? { start } : {}), + ...(end !== undefined ? { end } : {}), + }; +} + export function parseOmero( omeroData: Record | undefined | null, ): Omero | undefined { @@ -80,56 +117,17 @@ export function parseOmero( const channels: OmeroChannel[] = []; for (const channel of omeroData.channels as Array>) { - if ( - !channel || - typeof channel !== "object" || - !("window" in channel) || - !channel.window - ) { + if (!channel || typeof channel !== "object") { + channels.push({}); continue; } - const windowData = channel.window as Record; - if (typeof windowData !== "object") { - continue; - } - - // Handle backward compatibility for OMERO window metadata - // Prefer start/end format, fall back to min/max, use one as the other if needed - let start: number; - let end: number; - let minVal: number; - let maxVal: number; - - if ("start" in windowData && "end" in windowData) { - // New format with start/end - start = Number(windowData.start); - end = Number(windowData.end); - // Use start/end as min/max if not present - minVal = ("min" in windowData) ? Number(windowData.min) : start; - maxVal = ("max" in windowData) ? Number(windowData.max) : end; - } else if ("min" in windowData && "max" in windowData) { - // Old format with min/max only - minVal = Number(windowData.min); - maxVal = Number(windowData.max); - // Use min/max as start/end for backward compatibility - start = minVal; - end = maxVal; - } else { - // Invalid window data, skip this channel - continue; - } - - const window: OmeroWindow = { - min: minVal, - max: maxVal, - start: start, - end: end, - }; - + const window = parseOmeroWindow(channel.window); const omeroChannel: OmeroChannel = { - color: String(channel.color), - window, + ...(channel.color !== undefined && channel.color !== null + ? { color: String(channel.color) } + : {}), + ...(window !== undefined ? { window } : {}), ...(channel.label !== undefined && channel.label !== null ? { label: String(channel.label) } : {}), @@ -141,10 +139,6 @@ export function parseOmero( channels.push(omeroChannel); } - if (channels.length === 0) { - return undefined; - } - return { channels, ...(typeof omeroData.version === "string" diff --git a/ts/src/utils/structural_validation.ts b/ts/src/utils/structural_validation.ts index c605fd9b..16428d98 100644 --- a/ts/src/utils/structural_validation.ts +++ b/ts/src/utils/structural_validation.ts @@ -643,8 +643,14 @@ export function validateOmeroColorHex(metadata: Metadata): void { return; } for (let i = 0; i < omero.channels.length; i++) { + const color = omero.channels[i].color; + // A channel with no color passes: whether the field may be absent is the + // schema's decision, not this rule's. + if (color === undefined) { + continue; + } try { - validateColor(omero.channels[i].color); + validateColor(color); } catch (error) { throw new ValidationError( SpecRule.OmeroChannelColorFormat, diff --git a/ts/test/compute_omero_test.ts b/ts/test/compute_omero_test.ts index b2f9bb59..9f7bac9b 100644 --- a/ts/test/compute_omero_test.ts +++ b/ts/test/compute_omero_test.ts @@ -89,15 +89,15 @@ Deno.test("compute basic statistics for single channel", async () => { const channel = omero.channels[0]; // Check min/max - assertEquals(channel.window.min, 0); - assertEquals(channel.window.max, 99); + assertEquals(channel.window!.min, 0); + assertEquals(channel.window!.max, 99); // Check quantiles are approximately correct - assertExists(channel.window.start); - assertExists(channel.window.end); + assertExists(channel.window!.start); + assertExists(channel.window!.end); // 2% of 100 values is ~2, 98% is ~97 - assertEquals(channel.window.start! >= 0 && channel.window.start! <= 5, true); - assertEquals(channel.window.end! >= 94 && channel.window.end! <= 99, true); + assertEquals(channel.window!.start >= 0 && channel.window!.start <= 5, true); + assertEquals(channel.window!.end >= 94 && channel.window!.end <= 99, true); }); Deno.test("single channel uses white color", async () => { @@ -121,10 +121,10 @@ Deno.test("custom quantiles are respected", async () => { const channel = omero.channels[0]; // 10% of 100 values is ~10, 90% is ~90 assertEquals( - channel.window.start! >= 5 && channel.window.start! <= 15, + channel.window!.start! >= 5 && channel.window!.start! <= 15, true, ); - assertEquals(channel.window.end! >= 85 && channel.window.end! <= 95, true); + assertEquals(channel.window!.end! >= 85 && channel.window!.end! <= 95, true); }); Deno.test("custom color is applied", async () => { @@ -176,16 +176,16 @@ Deno.test("per-channel statistics are computed", async () => { assertEquals(omero.channels.length, 3); // Channel 0: all zeros - assertEquals(omero.channels[0].window.min, 0); - assertEquals(omero.channels[0].window.max, 0); + assertEquals(omero.channels[0].window!.min, 0); + assertEquals(omero.channels[0].window!.max, 0); // Channel 1: all 100s - assertEquals(omero.channels[1].window.min, 100); - assertEquals(omero.channels[1].window.max, 100); + assertEquals(omero.channels[1].window!.min, 100); + assertEquals(omero.channels[1].window!.max, 100); // Channel 2: all 255s - assertEquals(omero.channels[2].window.min, 255); - assertEquals(omero.channels[2].window.max, 255); + assertEquals(omero.channels[2].window!.min, 255); + assertEquals(omero.channels[2].window!.max, 255); }); Deno.test("multi-channel uses glasbey colors", async () => { @@ -288,8 +288,8 @@ Deno.test("3D image without channel dimension", async () => { const omero = await computeOmeroFromNgffImage(image); assertEquals(omero.channels.length, 1); - assertEquals(omero.channels[0].window.min, 0); - assertEquals(omero.channels[0].window.max, 999); + assertEquals(omero.channels[0].window!.min, 0); + assertEquals(omero.channels[0].window!.max, 999); }); Deno.test("4D image with channel dimension", async () => { @@ -308,10 +308,10 @@ Deno.test("4D image with channel dimension", async () => { const omero = await computeOmeroFromNgffImage(image); assertEquals(omero.channels.length, 2); - assertEquals(omero.channels[0].window.min, 0); - assertEquals(omero.channels[0].window.max, 0); - assertEquals(omero.channels[1].window.min, 100); - assertEquals(omero.channels[1].window.max, 100); + assertEquals(omero.channels[0].window!.min, 0); + assertEquals(omero.channels[0].window!.max, 0); + assertEquals(omero.channels[1].window!.min, 100); + assertEquals(omero.channels[1].window!.max, 100); }); // ============================================================================ @@ -325,8 +325,8 @@ Deno.test("handles NaN values", async () => { const omero = await computeOmeroFromNgffImage(image); // Should ignore NaN values - assertEquals(omero.channels[0].window.min, 1); - assertEquals(omero.channels[0].window.max, 9); + assertEquals(omero.channels[0].window!.min, 1); + assertEquals(omero.channels[0].window!.max, 9); }); Deno.test("constant value array", async () => { @@ -335,10 +335,10 @@ Deno.test("constant value array", async () => { const omero = await computeOmeroFromNgffImage(image); - assertEquals(omero.channels[0].window.min, 42); - assertEquals(omero.channels[0].window.max, 42); - assertEquals(omero.channels[0].window.start, 42); - assertEquals(omero.channels[0].window.end, 42); + assertEquals(omero.channels[0].window!.min, 42); + assertEquals(omero.channels[0].window!.max, 42); + assertEquals(omero.channels[0].window!.start, 42); + assertEquals(omero.channels[0].window!.end, 42); }); Deno.test("integer dtype works correctly", async () => { @@ -347,8 +347,8 @@ Deno.test("integer dtype works correctly", async () => { const omero = await computeOmeroFromNgffImage(image); - assertEquals(omero.channels[0].window.min, 0); - assertEquals(omero.channels[0].window.max, 255); + assertEquals(omero.channels[0].window!.min, 0); + assertEquals(omero.channels[0].window!.max, 255); }); // ============================================================================ @@ -378,8 +378,8 @@ Deno.test("computeOmeroFromMultiscales uses highest resolution", async () => { assertEquals(omero.channels.length, 1); // Should have original full-resolution values - assertEquals(omero.channels[0].window.min, 0); - assertEquals(omero.channels[0].window.max, 63); + assertEquals(omero.channels[0].window!.min, 0); + assertEquals(omero.channels[0].window!.max, 63); }); Deno.test("computeOmeroFromMultiscales passes through options", async () => { diff --git a/ts/test/from_ngff_zarr_test.ts b/ts/test/from_ngff_zarr_test.ts index cfe89dd5..7af59d21 100644 --- a/ts/test/from_ngff_zarr_test.ts +++ b/ts/test/from_ngff_zarr_test.ts @@ -310,11 +310,11 @@ Deno.test("omero metadata backward compatibility", async () => { // Check that the window has both min/max and start/end populated const channel = multiscales.metadata.omero.channels[0]; - assertEquals(channel.window.min, 0); - assertEquals(channel.window.max, 1000); + assertEquals(channel.window!.min, 0); + assertEquals(channel.window!.max, 1000); // For backward compatibility, min/max should be used as start/end - assertEquals(channel.window.start, 0); - assertEquals(channel.window.end, 1000); + assertEquals(channel.window!.start, 0); + assertEquals(channel.window!.end, 1000); // Test with start/end format only (newer format) const store2: MemoryStore = new Map(); @@ -397,10 +397,10 @@ Deno.test("omero metadata backward compatibility", async () => { assertExists(multiscales2.metadata.omero); const channel2 = multiscales2.metadata.omero.channels[0]; // For forward compatibility, start/end should be used as min/max - assertEquals(channel2.window.start, 10); - assertEquals(channel2.window.end, 900); - assertEquals(channel2.window.min, 10); - assertEquals(channel2.window.max, 900); + assertEquals(channel2.window!.start, 10); + assertEquals(channel2.window!.end, 900); + assertEquals(channel2.window!.min, 10); + assertEquals(channel2.window!.max, 900); // Test with both formats present (most complete) const store3: MemoryStore = new Map(); @@ -483,10 +483,10 @@ Deno.test("omero metadata backward compatibility", async () => { assertExists(multiscales3); assertExists(multiscales3.metadata.omero); const channel3 = multiscales3.metadata.omero.channels[0]; - assertEquals(channel3.window.min, 5); - assertEquals(channel3.window.max, 995); - assertEquals(channel3.window.start, 15); - assertEquals(channel3.window.end, 985); + assertEquals(channel3.window!.min, 5); + assertEquals(channel3.window!.max, 995); + assertEquals(channel3.window!.start, 15); + assertEquals(channel3.window!.end, 985); console.log("✓ OMERO metadata backward compatibility test passed"); }); diff --git a/ts/test/model_matches_schema_test.ts b/ts/test/model_matches_schema_test.ts new file mode 100644 index 00000000..6f236a00 --- /dev/null +++ b/ts/test/model_matches_schema_test.ts @@ -0,0 +1,72 @@ +/** + * The TypeScript types accept what the bundled schema of their version declares, + * and the structural rules treat an absent field the way the Python port does. + */ +import { assertEquals, assertThrows } from "@std/assert"; +import { parseOmero } from "../src/utils/parse_metadata.ts"; +import { + validateAxisOrder, + validateOmeroColorHex, +} from "../src/utils/structural_validation.ts"; +import type { Metadata } from "../src/types/zarr_metadata.ts"; + +Deno.test("every omero channel keeps its place", () => { + const omero = parseOmero({ + version: "0.4", + channels: [ + { + color: "FF0000", + window: { min: 0, max: 255, start: 0, end: 255 }, + label: "first", + active: true, + }, + { color: "00FF00", label: "no window" }, + { window: { min: 0, max: 1 }, label: "no color" }, + ], + }); + + assertEquals(omero?.channels.map((c) => c.label), [ + "first", + "no window", + "no color", + ]); + assertEquals(omero?.version, "0.4"); + assertEquals(omero?.channels[0].active, true); + assertEquals(omero?.channels[1].window, undefined); + assertEquals(omero?.channels[2].color, undefined); + // min/max standing in for start/end, as this library has always read them + assertEquals(omero?.channels[2].window, { min: 0, max: 1, start: 0, end: 1 }); +}); + +Deno.test("a channel without a color passes the color rule", () => { + const metadata = { + omero: { channels: [{ label: "no color" }] }, + } as unknown as Metadata; + validateOmeroColorHex(metadata); + + const bad = { + omero: { channels: [{ color: "nope" }] }, + } as unknown as Metadata; + assertThrows(() => validateOmeroColorHex(bad)); +}); + +Deno.test("an axis with no type carries no ordering constraint", () => { + const metadata = { + axes: [ + { name: "x", type: "space", unit: undefined }, + { name: "t", unit: undefined }, + { name: "c", type: "channel", unit: undefined }, + ], + datasets: [], + } as unknown as Metadata; + validateAxisOrder(metadata); + + const ordered = { + axes: [ + { name: "x", type: "space", unit: undefined }, + { name: "c", type: "channel", unit: undefined }, + ], + datasets: [], + } as unknown as Metadata; + assertThrows(() => validateAxisOrder(ordered)); +}); diff --git a/ts/test/omero_channel_chunking_test.ts b/ts/test/omero_channel_chunking_test.ts index a1833e40..1d815096 100644 --- a/ts/test/omero_channel_chunking_test.ts +++ b/ts/test/omero_channel_chunking_test.ts @@ -90,7 +90,7 @@ for (const cChunk of [1, 2, N_CHANNELS]) { assertEquals(omero.channels.length, N_CHANNELS); for (const [c, [min, max]] of expectedWindows().entries()) { assertEquals( - [omero.channels[c].window.min, omero.channels[c].window.max], + [omero.channels[c].window!.min, omero.channels[c].window!.max], [min, max], `channel ${c} statistics are wrong for chunk size ${cChunk}`, ); diff --git a/ts/test/omero_test.ts b/ts/test/omero_test.ts index b3bca498..3caf8ec9 100644 --- a/ts/test/omero_test.ts +++ b/ts/test/omero_test.ts @@ -34,50 +34,50 @@ Deno.test("read omero metadata from test dataset", async () => { // Channel 0 assertEquals(omero.channels[0].color, "FFFFFF"); - assertEquals(omero.channels[0].window.min, 0.0); - assertEquals(omero.channels[0].window.max, 65535.0); - assertEquals(omero.channels[0].window.start, 0.0); - assertEquals(omero.channels[0].window.end, 1200.0); + assertEquals(omero.channels[0].window!.min, 0.0); + assertEquals(omero.channels[0].window!.max, 65535.0); + assertEquals(omero.channels[0].window!.start, 0.0); + assertEquals(omero.channels[0].window!.end, 1200.0); assertEquals(omero.channels[0].label, "cy 1"); // Channel 1 assertEquals(omero.channels[1].color, "FFFFFF"); - assertEquals(omero.channels[1].window.min, 0.0); - assertEquals(omero.channels[1].window.max, 65535.0); - assertEquals(omero.channels[1].window.start, 0.0); - assertEquals(omero.channels[1].window.end, 1200.0); + assertEquals(omero.channels[1].window!.min, 0.0); + assertEquals(omero.channels[1].window!.max, 65535.0); + assertEquals(omero.channels[1].window!.start, 0.0); + assertEquals(omero.channels[1].window!.end, 1200.0); assertEquals(omero.channels[1].label, "cy 2"); // Channel 2 assertEquals(omero.channels[2].color, "FFFFFF"); - assertEquals(omero.channels[2].window.min, 0.0); - assertEquals(omero.channels[2].window.max, 65535.0); - assertEquals(omero.channels[2].window.start, 0.0); - assertEquals(omero.channels[2].window.end, 1200.0); + assertEquals(omero.channels[2].window!.min, 0.0); + assertEquals(omero.channels[2].window!.max, 65535.0); + assertEquals(omero.channels[2].window!.start, 0.0); + assertEquals(omero.channels[2].window!.end, 1200.0); assertEquals(omero.channels[2].label, "cy 3"); // Channel 3 assertEquals(omero.channels[3].color, "FFFFFF"); - assertEquals(omero.channels[3].window.min, 0.0); - assertEquals(omero.channels[3].window.max, 65535.0); - assertEquals(omero.channels[3].window.start, 0.0); - assertEquals(omero.channels[3].window.end, 1200.0); + assertEquals(omero.channels[3].window!.min, 0.0); + assertEquals(omero.channels[3].window!.max, 65535.0); + assertEquals(omero.channels[3].window!.start, 0.0); + assertEquals(omero.channels[3].window!.end, 1200.0); assertEquals(omero.channels[3].label, "cy 4"); // Channel 4 assertEquals(omero.channels[4].color, "0000FF"); - assertEquals(omero.channels[4].window.min, 0.0); - assertEquals(omero.channels[4].window.max, 65535.0); - assertEquals(omero.channels[4].window.start, 0.0); - assertEquals(omero.channels[4].window.end, 5000.0); + assertEquals(omero.channels[4].window!.min, 0.0); + assertEquals(omero.channels[4].window!.max, 65535.0); + assertEquals(omero.channels[4].window!.start, 0.0); + assertEquals(omero.channels[4].window!.end, 5000.0); assertEquals(omero.channels[4].label, "DAPI"); // Channel 5 assertEquals(omero.channels[5].color, "FF0000"); - assertEquals(omero.channels[5].window.min, 0.0); - assertEquals(omero.channels[5].window.max, 65535.0); - assertEquals(omero.channels[5].window.start, 0.0); - assertEquals(omero.channels[5].window.end, 100.0); + assertEquals(omero.channels[5].window!.min, 0.0); + assertEquals(omero.channels[5].window!.max, 65535.0); + assertEquals(omero.channels[5].window!.start, 0.0); + assertEquals(omero.channels[5].window!.end, 100.0); assertEquals(omero.channels[5].label, "Hyb probe"); }); @@ -168,12 +168,12 @@ Deno.test("write omero metadata", async () => { assertExists(readOmero); assertEquals(readOmero.channels.length, 2); assertEquals(readOmero.channels[0].color, "008000"); - assertEquals(readOmero.channels[0].window.start, 10.0); - assertEquals(readOmero.channels[0].window.end, 150.0); + assertEquals(readOmero.channels[0].window!.start, 10.0); + assertEquals(readOmero.channels[0].window!.end, 150.0); assertEquals(readOmero.channels[0].label, "Phalloidin"); assertEquals(readOmero.channels[1].color, "0000FF"); - assertEquals(readOmero.channels[1].window.start, 30.0); - assertEquals(readOmero.channels[1].window.end, 200.0); + assertEquals(readOmero.channels[1].window!.start, 30.0); + assertEquals(readOmero.channels[1].window!.end, 200.0); assertEquals(readOmero.channels[1].label, ""); }); @@ -392,12 +392,12 @@ Deno.test("write omero metadata v0.5 - omero inside ome namespace", async () => assertExists(readOmero); assertEquals(readOmero.channels.length, 2); assertEquals(readOmero.channels[0].color, "a52a2a"); - assertEquals(readOmero.channels[0].window.start, 0.0); - assertEquals(readOmero.channels[0].window.end, 255.0); + assertEquals(readOmero.channels[0].window!.start, 0.0); + assertEquals(readOmero.channels[0].window!.end, 255.0); assertEquals(readOmero.channels[0].label, "C1-DAPI"); assertEquals(readOmero.channels[1].color, "00aeef"); - assertEquals(readOmero.channels[1].window.start, 0.0); - assertEquals(readOmero.channels[1].window.end, 255.0); + assertEquals(readOmero.channels[1].window!.start, 0.0); + assertEquals(readOmero.channels[1].window!.end, 255.0); assertEquals(readOmero.channels[1].label, "C2-Phalloidin"); }); @@ -442,14 +442,14 @@ Deno.test("create omero channel and window", () => { }; assertEquals(channel.color, "FF0000"); - assertEquals(channel.window.min, 0.0); - assertEquals(channel.window.max, 255.0); - assertEquals(channel.window.start, 10.0); - assertEquals(channel.window.end, 200.0); + assertEquals(channel.window!.min, 0.0); + assertEquals(channel.window!.max, 255.0); + assertEquals(channel.window!.start, 10.0); + assertEquals(channel.window!.end, 200.0); assertEquals(channel.label, "Red Channel"); // Test validation - validateColor(channel.color); + validateColor(channel.color!); }); Deno.test("create omero metadata with multiple channels", () => { @@ -482,5 +482,5 @@ Deno.test("create omero metadata with multiple channels", () => { assertEquals(omero.channels[2].label, "Blue"); // Validate all colors - omero.channels.forEach((channel) => validateColor(channel.color)); + omero.channels.forEach((channel) => validateColor(channel.color!)); }); From 4f385e809ab936c2cc91c11454e63d924b719a85 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 10:48:31 +0200 Subject: [PATCH 02/10] fix(py): make validate=True mean the same thing at every version The 0.6 reader ran the schema pass alone, so a store whose datasets run coarsest to finest was refused at 0.4 and 0.5 and accepted at 0.6: the meaning of the flag followed the version of the store. The 0.6 reader now runs the structural pass after the schema pass, as the 0.4 and 0.5 readers do. The default validate=False path is unchanged. from_hcs_zarr(validate=True) checked the plate and the well metadata and then read every image with a bare from_ome_zarr, so the images themselves were never checked. The flag now reaches them. _intrinsic_system said it mirrors Metadata.intrinsic_coordinate_system, which it does on a well-formed document and not on a malformed one: it answers with the first listed system wherever that property raises. That tolerance is what lets a malformed document reach the validator instead of crashing ahead of it, so the docstring says so. Ref #667. --- py/ngff_zarr/hcs.py | 31 ++++++-- py/ngff_zarr/parse_metadata.py | 11 ++- py/ngff_zarr/v06/zarr_metadata.py | 13 ++++ py/test/test_hcs_validate_reaches_images.py | 76 +++++++++++++++++++ .../test_validate_is_version_independent.py | 74 ++++++++++++++++++ 5 files changed, 197 insertions(+), 8 deletions(-) create mode 100644 py/test/test_hcs_validate_reaches_images.py create mode 100644 py/test/test_validate_is_version_independent.py diff --git a/py/ngff_zarr/hcs.py b/py/ngff_zarr/hcs.py index 860dfbaa..272d6427 100644 --- a/py/ngff_zarr/hcs.py +++ b/py/ngff_zarr/hcs.py @@ -162,7 +162,11 @@ def get_well(self, row_name: str, column_name: str) -> Optional["HCSWell"]: # Cache wells to avoid reloading - using bounded cache now if well_path not in self._wells: hcs_well = HCSWell.from_store( - self.store, well_path, well_meta, self.image_cache_size + self.store, + well_path, + well_meta, + self.image_cache_size, + validate=self._validate, ) if self._validate: # Strict structural validation of the well's own metadata, in @@ -215,11 +219,13 @@ def __init__( well_metadata: PlateWell, well_group_metadata: Well, image_cache_size: int | None = None, + validate: bool = False, ): self.store = store self.path = well_path self.plate_metadata = well_metadata self.metadata = well_group_metadata + self._validate = validate # Use bounded cache for images to prevent memory issues from .config import config @@ -234,8 +240,14 @@ def from_store( well_path: str, well_metadata: PlateWell, image_cache_size: int | None = None, + validate: bool = False, ) -> "HCSWell": - """Load a well from a zarr store.""" + """Load a well from a zarr store. + + ``validate`` reaches the images this well reads: ``from_hcs_zarr + (validate=True)`` checks the plate and the well metadata, and the + images are part of what it was asked to check. + """ # Dispatches on store type: local paths read through the compat # layer, bytes mappings (including ZipReadStore for .ozx) through # the store reader, other store objects through zarr-python. @@ -288,7 +300,12 @@ def from_store( well_group_metadata = Well(images=images, version=version) return cls( - store, well_path, well_metadata, well_group_metadata, image_cache_size + store, + well_path, + well_metadata, + well_group_metadata, + image_cache_size, + validate=validate, ) @property @@ -317,18 +334,20 @@ def get_image(self, field_index: int = 0) -> NgffMultiscales | None: # A view of the same remote store narrowed to this field's # sub-hierarchy, sharing the obstore client. self._images[image_path] = from_ome_zarr( - self.store.with_prefix(image_path) + self.store.with_prefix(image_path), validate=self._validate ) elif isinstance(self.store, ZipReadStore): # A view of the same archive narrowed to this field's # sub-hierarchy; from_ome_zarr reads it as a mapping store. self._images[image_path] = from_ome_zarr( - self.store.with_prefix(image_path) + self.store.with_prefix(image_path), validate=self._validate ) elif isinstance(self.store, (str, Path)): # If store is a path string, append the image path full_image_path = Path(self.store) / self.path / image_meta.path - self._images[image_path] = from_ome_zarr(str(full_image_path)) + self._images[image_path] = from_ome_zarr( + str(full_image_path), validate=self._validate + ) else: raise TypeError( "HCS plates are read from local directory paths, remote " diff --git a/py/ngff_zarr/parse_metadata.py b/py/ngff_zarr/parse_metadata.py index c97f1835..a433937a 100644 --- a/py/ngff_zarr/parse_metadata.py +++ b/py/ngff_zarr/parse_metadata.py @@ -182,8 +182,15 @@ def _raw_axes(multiscales_entry: dict) -> list: def _intrinsic_system(multiscales_entry: dict, systems: list): """The coordinate system the datasets map into, else the first listed. - Mirrors :attr:`ngff_zarr.v06.zarr_metadata.Metadata.intrinsic_coordinate_system` - at the dict level, where the parsed dataclasses are not available yet. + The dict-level counterpart of + :attr:`ngff_zarr.v06.zarr_metadata.Metadata.intrinsic_coordinate_system`, + used where the parsed dataclasses are not available yet. It answers with + the first listed system wherever that property raises: a dataset with no + ``output``, an ``output`` naming a system the document does not define, or + no datasets at all. The two therefore agree on a well-formed document and + part company on a malformed one, by design. This runs ahead of validation, + on the raw dict, so that a malformed document reaches the validator and is + reported rather than crashing here. """ for dataset in multiscales_entry.get("datasets") or []: if not isinstance(dataset, dict): diff --git a/py/ngff_zarr/v06/zarr_metadata.py b/py/ngff_zarr/v06/zarr_metadata.py index c2c8c300..b9cce5a7 100644 --- a/py/ngff_zarr/v06/zarr_metadata.py +++ b/py/ngff_zarr/v06/zarr_metadata.py @@ -924,6 +924,19 @@ def _from_zarr_attrs( coordinateTransformations=additionalTransformations, ) + if validate: + # The structural pass, layered after the schema pass above, as the + # 0.4 and 0.5 readers do, so validate=True means the same thing at + # every version. Imported lazily so the default validate=False read + # path incurs no extra import cost. + from ..structural_validation import ( + ValidateOptions, + ValidationLevel, + validate_structural, + ) + + validate_structural(metadata, ValidateOptions(level=ValidationLevel.STRICT)) + return metadata, images @classmethod diff --git a/py/test/test_hcs_validate_reaches_images.py b/py/test/test_hcs_validate_reaches_images.py new file mode 100644 index 00000000..4cd532e0 --- /dev/null +++ b/py/test/test_hcs_validate_reaches_images.py @@ -0,0 +1,76 @@ +# SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +# SPDX-License-Identifier: MIT +"""``from_hcs_zarr(validate=True)`` checks the images, not only the plate.""" + +import json +from pathlib import Path + +import ngff_zarr as nz +import numpy as np +import pytest +from ngff_zarr import ValidationError +from ngff_zarr.v04.zarr_metadata import Plate, PlateColumn, PlateRow, PlateWell + + +def _plate(tmp_path, version="0.5"): + store = tmp_path / "plate.ome.zarr" + plate_metadata = Plate( + columns=[PlateColumn(name="1")], + rows=[PlateRow(name="A")], + wells=[PlateWell(path="A/1", rowIndex=0, columnIndex=0)], + version=version, + ) + multiscales = nz.to_multiscales( + np.zeros((16, 16), dtype=np.uint8), scale_factors=[2] + ) + nz.write_hcs_well_image( + store=str(store), + multiscales=multiscales, + plate_metadata=plate_metadata, + row_name="A", + column_name="1", + field_index=0, + version=version, + ) + # write_hcs_well_image writes the well and its image; the plate root is + # published separately, and from_hcs_zarr reads the wells from it. + nz.to_hcs_zarr( + nz.HCSPlate(store=str(store), plate_metadata=plate_metadata), + str(store), + overwrite=False, + ) + return store + + +def _reverse_image_datasets(store, version="0.5"): + path = ( + Path(store) / "A" / "1" / "0" / ("zarr.json" if version != "0.4" else ".zattrs") + ) + doc = json.loads(path.read_text()) + entry = doc["attributes"]["ome"] if version != "0.4" else doc + entry["multiscales"][0]["datasets"].reverse() + path.write_text(json.dumps(doc, indent=4)) + + +def test_a_valid_plate_reads_with_validate(tmp_path): + store = _plate(tmp_path) + plate = nz.from_hcs_zarr(str(store), validate=True) + assert plate.get_well("A", "1").get_image(0) is not None + + +def test_a_broken_image_is_reported(tmp_path): + """The flag is accepted for the whole plate, so it has to reach the images.""" + store = _plate(tmp_path) + _reverse_image_datasets(store) + + plate = nz.from_hcs_zarr(str(store), validate=True) + with pytest.raises((ValidationError, ValueError)): + plate.get_well("A", "1").get_image(0) + + +def test_the_default_still_reads_a_broken_image(tmp_path): + store = _plate(tmp_path) + _reverse_image_datasets(store) + + plate = nz.from_hcs_zarr(str(store)) + assert plate.get_well("A", "1").get_image(0) is not None diff --git a/py/test/test_validate_is_version_independent.py b/py/test/test_validate_is_version_independent.py new file mode 100644 index 00000000..d65297ac --- /dev/null +++ b/py/test/test_validate_is_version_independent.py @@ -0,0 +1,74 @@ +# SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +# SPDX-License-Identifier: MIT +"""``validate=True`` means the same thing whatever the version of the store.""" + +import json +from pathlib import Path + +import dask.array as da +import numpy as np +import pytest +from ngff_zarr import ( + NgffImage, + ValidationError, + from_ome_zarr, + to_multiscales, + to_ome_zarr, +) + +VERSIONS = ["0.4", "0.5", "0.6"] + + +def _store(tmp_path, version): + image = NgffImage( + da.zeros((1, 8, 8), dtype=np.uint8, chunks=(1, 8, 8)), + ["c", "y", "x"], + {"c": 1.0, "y": 1.0, "x": 1.0}, + {"c": 0.0, "y": 0.0, "x": 0.0}, + ) + store = str(tmp_path / f"v{version}.ome.zarr") + to_ome_zarr( + store, to_multiscales(image, scale_factors=[2], cache=False), version=version + ) + return store + + +def _entry_path(store, version): + return Path(store) / (".zattrs" if version == "0.4" else "zarr.json") + + +def _reverse_datasets(store, version): + path = _entry_path(store, version) + doc = json.loads(path.read_text()) + entry = doc if version == "0.4" else doc["attributes"]["ome"] + entry["multiscales"][0]["datasets"].reverse() + path.write_text(json.dumps(doc, indent=4)) + + +@pytest.mark.parametrize("version", VERSIONS) +def test_a_valid_store_reads_with_validate(tmp_path, version): + store = _store(tmp_path, version) + assert len(from_ome_zarr(store, validate=True).images) == 2 + + +@pytest.mark.parametrize("version", VERSIONS) +def test_datasets_coarsest_to_finest_are_refused_at_every_version(tmp_path, version): + """The structural rules run at 0.6 as they do at 0.4 and 0.5. + + Before this, a 0.6 store went through the schema pass alone, so the + meaning of ``validate=True`` followed the version of the store. + """ + store = _store(tmp_path, version) + _reverse_datasets(store, version) + + with pytest.raises((ValidationError, ValueError)): + from_ome_zarr(store, validate=True) + + +@pytest.mark.parametrize("version", VERSIONS) +def test_the_same_store_still_reads_without_validate(tmp_path, version): + """The default read path is unchanged: the rules run only when asked.""" + store = _store(tmp_path, version) + _reverse_datasets(store, version) + + assert len(from_ome_zarr(store).images) == 2 From 5fecc67eba11700d09af5bd9946183247ee00ba0 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 12:20:51 +0200 Subject: [PATCH 03/10] fix(mcp): report a store the spec refuses as invalid validate_ome_zarr ran one schema pass and demoted its failure to a warning, so an invalid document came back valid=True. It also never ran the structural rules, so a store whose datasets go coarsest to finest passed. Both verdicts are errors now and reach the caller through valid. The import fallback was a silent no-op: without the ngff-zarr[validate] extra every store was declared valid on the strength of a check that never ran. It now says the schema pass did not run. --- mcp/ngff_zarr_mcp/tools.py | 49 +++++++++++----- mcp/tests/test_validate_ome_zarr.py | 87 +++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 mcp/tests/test_validate_ome_zarr.py diff --git a/mcp/ngff_zarr_mcp/tools.py b/mcp/ngff_zarr_mcp/tools.py index ba798fc8..a9afc4af 100644 --- a/mcp/ngff_zarr_mcp/tools.py +++ b/mcp/ngff_zarr_mcp/tools.py @@ -8,26 +8,22 @@ from ngff_zarr import ( # type: ignore[import-untyped] Methods, + ValidationError, cli_input_to_ngff_image, detect_cli_io_backend, from_ome_zarr, to_multiscales, to_ome_zarr, + validate_structural, ) -# Import validation function if available +# The schema pass needs the ngff-zarr[validate] extra. Where it is missing the +# tool says the pass did not run, rather than calling a store valid on the +# strength of a check it never made. try: from ngff_zarr import validate as validate_ngff except ImportError: - # Fallback if ngff-zarr schema validation is unavailable (no-op stub whose - # signature mirrors ngff_zarr.validate). - def validate_ngff( - ngff_dict: dict, - version: str = "0.4", - model: str = "image", - strict: bool = False, - ) -> None: - pass + validate_ngff = None from .models import ( @@ -319,6 +315,12 @@ async def inspect_ome_zarr(store_path: str) -> StoreInfo: raise ValueError(f"Failed to inspect store: {str(e)}") +_SCHEMA_PASS_SKIPPED = ( + "Schema validation did not run: install the ngff-zarr[validate] extra to " + "check the metadata document against its JSON Schema." +) + + async def validate_ome_zarr(store_path: str) -> ValidationResult: """Validate an OME-Zarr store.""" @@ -378,12 +380,29 @@ async def validate_ome_zarr(store_path: str) -> ValidationResult: except Exception: version = "0.4" # Default assumption - # Try ngff-zarr schema validation if available. ngff_zarr.validate - # expects the parsed NGFF metadata dict, not a store path. + # ngff_zarr.validate expects the parsed NGFF metadata dict, not a + # store path. A document the schema rejects is invalid, so the + # verdict belongs in errors and reaches the caller through `valid`. + if validate_ngff is None: + warnings.append(_SCHEMA_PASS_SKIPPED) + else: + try: + validate_ngff(root_attrs, version=version or "0.4") + except ImportError: + warnings.append(_SCHEMA_PASS_SKIPPED) + except Exception as validation_error: + errors.append(f"Schema validation failed: {validation_error}") + + # The structural rules carry the spec MUSTs no JSON Schema states, + # among them the finest-to-coarsest dataset order. They read the + # parsed model, so they run on the multiscales loaded above. try: - validate_ngff(root_attrs, version=version or "0.4") - except Exception as validation_error: - warnings.append(f"NGFF validation warning: {str(validation_error)}") + validate_structural(multiscales.metadata) + except ValidationError as structural_error: + errors.append(f"Structural validation failed: {structural_error}") + except ImportError: + # One rule, the RFC 4 orientation check, reaches for jsonschema. + warnings.append(_SCHEMA_PASS_SKIPPED) except Exception as e: errors.append(f"Failed to load as NGFF: {str(e)}") diff --git a/mcp/tests/test_validate_ome_zarr.py b/mcp/tests/test_validate_ome_zarr.py new file mode 100644 index 00000000..b99d9cc3 --- /dev/null +++ b/mcp/tests/test_validate_ome_zarr.py @@ -0,0 +1,87 @@ +# SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +# SPDX-License-Identifier: MIT +"""``validate_ome_zarr`` reports a store the spec refuses as invalid. + +The tool runs two passes over a store: the JSON Schema of the store's own +version, and the structural rules that carry the spec MUSTs no schema states. +A failure in either is an error, so it reaches the caller through ``valid``. +""" + +import json +from pathlib import Path + +import numpy as np +import pytest +from ngff_zarr import to_multiscales, to_ngff_image, to_ome_zarr + +from ngff_zarr_mcp.tools import validate_ome_zarr + +VERSIONS = ["0.4", "0.5", "0.6"] + + +def _write_store(path: Path, version: str) -> Path: + image = to_ngff_image( + np.zeros((32, 32), dtype=np.uint8), dims=["y", "x"], scale={"y": 1.0, "x": 1.0} + ) + multiscales = to_multiscales(image, scale_factors=[2]) + to_ome_zarr(str(path), multiscales, version=version) + return path + + +def _root_document(store: Path) -> tuple[Path, dict]: + """The root attributes document and its path, whichever layout is on disk.""" + v3 = store / "zarr.json" + if v3.exists(): + return v3, json.loads(v3.read_text()) + v2 = store / ".zattrs" + return v2, json.loads(v2.read_text()) + + +def _edit_multiscales(store: Path, mutate) -> None: + path, document = _root_document(store) + if "attributes" in document: + multiscales = document["attributes"]["ome"]["multiscales"] + else: + multiscales = document["multiscales"] + mutate(multiscales[0]) + path.write_text(json.dumps(document)) + consolidated = store / "zarr.json" + if consolidated.exists() and path == consolidated: + return + + +@pytest.mark.asyncio +@pytest.mark.parametrize("version", VERSIONS) +async def test_a_valid_store_is_valid_at_every_version(tmp_path, version): + """Zarr v3 stores, which is every 0.5 and 0.6 store, get past the store check.""" + store = _write_store(tmp_path / f"valid-{version}.zarr", version) + + result = await validate_ome_zarr(str(store)) + + assert result.valid, result.errors + + +@pytest.mark.asyncio +@pytest.mark.parametrize("version", VERSIONS) +async def test_datasets_ordered_coarsest_to_finest_are_refused(tmp_path, version): + """`dataset-order-highest-to-lowest`, a rule no JSON Schema states.""" + store = _write_store(tmp_path / f"reversed-{version}.zarr", version) + _edit_multiscales(store, lambda ms: ms["datasets"].reverse()) + + result = await validate_ome_zarr(str(store)) + + assert not result.valid + assert any("dataset" in error.lower() for error in result.errors), result.errors + + +@pytest.mark.asyncio +async def test_a_document_the_schema_rejects_is_an_error_not_a_warning(tmp_path): + """A schema failure used to be demoted to a warning, leaving `valid` True.""" + pytest.importorskip("jsonschema") + store = _write_store(tmp_path / "no-axes.zarr", "0.4") + _edit_multiscales(store, lambda ms: ms.pop("axes")) + + result = await validate_ome_zarr(str(store)) + + assert not result.valid + assert result.errors From edfc814cfd39957b450dda52f0614f9961c6182d Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 12:21:08 +0200 Subject: [PATCH 04/10] docs(py): correct two claims the code does not keep The extra field is documented as carried across version conversion. It is not: to_version starts the converted metadata with an empty extra, which test_clean_roundtrip_read_has_empty_extra already pins. The namespacing rules that read it run inside the parser of the version that captured it, before any conversion. The RFC 4 orientation schema reports nothing. Its root is an open object with every definition parked in $defs and no $ref reaching them, so an axis list of strings, an axes value that is not a list, and an orientation whose type and value are nonsense all pass. Upstream ome/ngff#585 and #586 describe the same two defects. Every rule that fires is the hand-written Python above the call, and test_the_bundled_rfc4_schema_checks_nothing pins that, so those checks are not later removed as redundant with a pass that does not run. --- py/ngff_zarr/rfc4_validation.py | 22 +++++--- py/ngff_zarr/v05/zarr_metadata.py | 9 ++-- py/ngff_zarr/v06/zarr_metadata.py | 9 ++-- py/test/test_rfc4_schema_is_inert.py | 77 ++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 13 deletions(-) create mode 100644 py/test/test_rfc4_schema_is_inert.py diff --git a/py/ngff_zarr/rfc4_validation.py b/py/ngff_zarr/rfc4_validation.py index f8427064..f08ded6b 100644 --- a/py/ngff_zarr/rfc4_validation.py +++ b/py/ngff_zarr/rfc4_validation.py @@ -186,18 +186,26 @@ def validate_rfc4_orientation(axes: list[dict[str, Any]]) -> None: if not has_orientation: return - # Load the schema and validate the overall structure + # The bundled artifact reports nothing. Its root is ``{"type": "object", + # "additionalProperties": true}`` with every definition parked in ``$defs`` + # and no ``$ref`` reaching them, so any JSON object satisfies it: an axis + # list of strings, an ``axes`` value that is not a list at all, and an + # orientation whose ``type`` and ``value`` are nonsense all pass. Upstream + # ome/ngff#585 and #586 describe the same two defects, the missing entry + # point and the unconstrained ``AnatomicalOrientation``. + # + # Every RFC 4 rule that fires is therefore the hand-written Python above, + # and none of it is redundant with this call. That is what + # ``test_the_bundled_rfc4_schema_checks_nothing`` pins, so the checks above + # cannot be deleted in favour of a schema pass that does not run. The call + # stays because it starts reporting the moment upstream gives the artifact + # a root entry point. schema = load_rfc4_orientation_schema() registry = Registry().with_resource( "https://w3id.org/ome/ngff", resource=Resource.from_contents(schema) ) validator = Draft202012Validator(schema, registry=registry) - - # Create a structure that matches the schema format - axes_structure = {"axes": axes} - - # Validate against the schema - validator.validate(axes_structure) + validator.validate({"axes": axes}) def has_any_rfc4_orientation(axes: list[dict[str, Any]]) -> bool: diff --git a/py/ngff_zarr/v05/zarr_metadata.py b/py/ngff_zarr/v05/zarr_metadata.py index abdd94bd..89fac96f 100644 --- a/py/ngff_zarr/v05/zarr_metadata.py +++ b/py/ngff_zarr/v05/zarr_metadata.py @@ -23,9 +23,12 @@ class Metadata: type: str | None = None metadata: MethodMetadata | None = None #: Unrecognized keys captured on read (see - #: :attr:`ngff_zarr.v04.zarr_metadata.Metadata.extra`). Carried across - #: version conversion so a value read back through ``to_version`` retains - #: the field; a read-side validation aid that is never serialized. + #: :attr:`ngff_zarr.v04.zarr_metadata.Metadata.extra`). A read-side + #: validation aid that is never serialized, and that ``to_version`` does + #: not carry: converted metadata starts with an empty ``extra``, which + #: ``test_clean_roundtrip_read_has_empty_extra`` pins. The namespacing + #: rules that read it run inside the parser of the version that captured + #: it, before any conversion. extra: dict = field(default_factory=dict) def to_version( diff --git a/py/ngff_zarr/v06/zarr_metadata.py b/py/ngff_zarr/v06/zarr_metadata.py index b9cce5a7..dd8b5329 100644 --- a/py/ngff_zarr/v06/zarr_metadata.py +++ b/py/ngff_zarr/v06/zarr_metadata.py @@ -531,9 +531,12 @@ class Metadata: type: str | None = None metadata: MethodMetadata | None = None #: Unrecognized keys captured on read (see - #: :attr:`ngff_zarr.v04.zarr_metadata.Metadata.extra`). Carried across - #: version conversion so a value read back through ``to_version`` retains - #: the field; a read-side validation aid that is never serialized. + #: :attr:`ngff_zarr.v04.zarr_metadata.Metadata.extra`). A read-side + #: validation aid that is never serialized, and that ``to_version`` does + #: not carry: converted metadata starts with an empty ``extra``, which + #: ``test_clean_roundtrip_read_has_empty_extra`` pins. The namespacing + #: rules that read it run inside the parser of the version that captured + #: it, before any conversion. extra: dict = field(default_factory=dict) def __post_init__(self): diff --git a/py/test/test_rfc4_schema_is_inert.py b/py/test/test_rfc4_schema_is_inert.py new file mode 100644 index 00000000..8749c1ef --- /dev/null +++ b/py/test/test_rfc4_schema_is_inert.py @@ -0,0 +1,77 @@ +# SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +# SPDX-License-Identifier: MIT +"""The bundled RFC 4 orientation schema reports nothing. + +``spec/rfc/4/orientation.schema.json`` has no root entry point: every +definition sits in ``$defs`` and nothing references them, so the closing +``validator.validate`` in :func:`ngff_zarr.rfc4_validation.validate_rfc4_orientation` +accepts any JSON object. Upstream ome/ngff#585 and #586 report the same. + +Every RFC 4 rule that fires is the hand-written Python in that function. These +tests pin that split so those checks are not later removed as redundant with a +schema pass that does not run, and so the day upstream publishes a schema with +a root entry point is visible as a failure here rather than a silent change. +""" + +import pytest +from ngff_zarr.rfc4_validation import load_rfc4_orientation_schema + +jsonschema = pytest.importorskip("jsonschema") + + +def _validator(): + from jsonschema import Draft202012Validator + from referencing import Registry, Resource + + schema = load_rfc4_orientation_schema() + registry = Registry().with_resource( + "https://w3id.org/ome/ngff", resource=Resource.from_contents(schema) + ) + return Draft202012Validator(schema, registry=registry) + + +def test_the_schema_has_no_root_entry_point(): + schema = load_rfc4_orientation_schema() + + assert "$defs" in schema + assert "properties" not in schema + assert "$ref" not in schema + assert schema.get("additionalProperties") is True + + +@pytest.mark.parametrize( + "document", + [ + {"axes": ["not an object", {"name": 42, "type": "nonsense"}]}, + {"axes": "not even a list"}, + {"axes": [{"orientation": {"type": "bogus", "value": "sideways"}}]}, + {"axes": [{"name": "x", "type": "space", "orientation": []}]}, + ], + ids=[ + "axis-is-a-string", + "axes-not-a-list", + "orientation-off-vocabulary", + "orientation-is-a-list", + ], +) +def test_the_bundled_rfc4_schema_checks_nothing(document): + assert list(_validator().iter_errors(document)) == [] + + +def test_the_hand_written_checks_are_what_reject_a_bad_orientation(): + """The same document the schema accepts is refused by the Python rules.""" + from ngff_zarr.rfc4_validation import validate_rfc4_orientation + + axes = [ + {"name": "y", "type": "space", "unit": "micrometer"}, + { + "name": "x", + "type": "space", + "unit": "micrometer", + "orientation": {"type": "bogus", "value": "left-to-right"}, + }, + ] + + assert list(_validator().iter_errors({"axes": axes})) == [] + with pytest.raises(ValueError, match="must be anatomical"): + validate_rfc4_orientation(axes) From 6655e4faa0c2c9bd79f41a7f5545c9b848e05111 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 12:21:57 +0200 Subject: [PATCH 05/10] fix(ts): stop measuring a v0.6 inter-system transform against the intrinsic axes At v0.6 a multiscale-level coordinateTransformations entry maps between two coordinate systems referenced by name, and spec/0.6/schemas/image.schema requires a name on both its input and its output. Its dimensionality follows those two systems. metadata.axes holds the intrinsic system's axes instead, so a legal document was rejected with scale-length-mismatch. The global arm is skipped when the metadata carries coordinateSystems, which is what the Python port already does by dropping those transforms in _flat_model. Per-dataset transforms do map an array to the intrinsic system, so their arm still runs at every version. --- ts/src/utils/structural_validation.ts | 13 +- ...al_validation_v06_global_transform_test.ts | 117 ++++++++++++++++++ 2 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 ts/test/structural_validation_v06_global_transform_test.ts diff --git a/ts/src/utils/structural_validation.ts b/ts/src/utils/structural_validation.ts index 16428d98..f1705dee 100644 --- a/ts/src/utils/structural_validation.ts +++ b/ts/src/utils/structural_validation.ts @@ -498,6 +498,15 @@ export function validatePerDatasetScaleCount(metadata: Metadata): void { * global-then-per-dataset, is reported. The length invariant itself lives in * {@link transformLenMismatch}. * + * The global arm is skipped when the metadata carries `coordinateSystems`, + * which marks the v0.6 model. There the multiscale-level + * `coordinateTransformations` map between two coordinate systems referenced by + * name, and their dimensionality follows those two systems rather than the + * intrinsic axis count that `metadata.axes` holds, so measuring them against + * `metadata.axes.length` rejects legal documents. The Python port drops the + * same transforms in `_flat_model` for the same reason. Per-dataset transforms + * do map the array to the intrinsic system, so their arm runs at every version. + * * @param metadata - The parsed multiscales metadata to validate. * @throws {ValidationError} With {@link SpecRule.ScaleLengthMismatch} for the * first transform whose vector length disagrees with `metadata.axes.length`; @@ -505,7 +514,9 @@ export function validatePerDatasetScaleCount(metadata: Metadata): void { */ export function validateScaleLength(metadata: Metadata): void { const axesLen = metadata.axes.length; - const globalTransforms = metadata.coordinateTransformations; + const globalTransforms = metadata.coordinateSystems + ? undefined + : metadata.coordinateTransformations; if (globalTransforms) { for (let j = 0; j < globalTransforms.length; j++) { const mismatch = transformLenMismatch(globalTransforms[j], axesLen); diff --git a/ts/test/structural_validation_v06_global_transform_test.ts b/ts/test/structural_validation_v06_global_transform_test.ts new file mode 100644 index 00000000..9b00fc19 --- /dev/null +++ b/ts/test/structural_validation_v06_global_transform_test.ts @@ -0,0 +1,117 @@ +// SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +// SPDX-License-Identifier: MIT +/** + * The `scale-length-mismatch` rule and v0.6 multiscale-level transforms. + * + * At v0.6 a multiscale-level `coordinateTransformations` entry maps between two + * coordinate systems referenced by name, and `spec/0.6/schemas/image.schema` + * requires a `name` on both its `input` and its `output`. Its dimensionality + * follows those two named systems. `metadata.axes` holds the intrinsic system's + * axes instead, so measuring such a transform against `metadata.axes.length` + * rejects a legal document. + * + * The Python port drops these transforms in `_flat_model` for the same reason, + * covered by `test_top_level_inter_system_transform_is_not_length_checked` in + * `py/test/test_structural_validation_v06.py`. This suite is its counterpart, + * so the two ports reach the same verdict on the same document. It also pins + * that per-dataset transforms, which do map an array to the intrinsic system, + * keep being measured at v0.6. + */ + +import { assertThrows } from "@std/assert"; +import { + SpecRule, + validateScaleLength, + validateStructural, + ValidationError, +} from "../src/utils/structural_validation.ts"; +import { + createScale, + type CoordinateSystem, + type Metadata, +} from "../src/types/zarr_metadata.ts"; + +/** Three intrinsic axes, plus two named 2D systems a transform maps between. */ +function coordinateSystems(): CoordinateSystem[] { + return [ + { + name: "intrinsic", + axes: [ + { name: "z", type: "space", unit: undefined }, + { name: "y", type: "space", unit: undefined }, + { name: "x", type: "space", unit: undefined }, + ], + }, + { + name: "slide", + axes: [ + { name: "y", type: "space", unit: undefined }, + { name: "x", type: "space", unit: undefined }, + ], + }, + { + name: "stage", + axes: [ + { name: "y", type: "space", unit: undefined }, + { name: "x", type: "space", unit: undefined }, + ], + }, + ]; +} + +/** + * A v0.6 document whose multiscale-level transform has two components against + * three intrinsic axes, which is what the two named systems it joins declare. + */ +function v06MetadataWithNamedSystemTransform(): Metadata { + const systems = coordinateSystems(); + return { + axes: systems[0].axes, + coordinateSystems: systems, + datasets: [ + { + path: "0", + coordinateTransformations: [createScale([1.0, 1.0, 1.0])], + }, + { + path: "1", + coordinateTransformations: [createScale([1.0, 2.0, 2.0])], + }, + ], + coordinateTransformations: [createScale([0.5, 0.5])], + omero: undefined, + name: "image", + version: "0.6", + }; +} + +Deno.test("a v0.6 transform between two named systems is not measured against the intrinsic axes", () => { + validateScaleLength(v06MetadataWithNamedSystemTransform()); + validateStructural(v06MetadataWithNamedSystemTransform()); +}); + +Deno.test("a v0.4 global transform is still measured against the axis count", () => { + const { coordinateSystems: _dropped, ...flat } = + v06MetadataWithNamedSystemTransform(); + const metadata: Metadata = { ...flat, version: "0.4" }; + + assertThrows( + () => validateScaleLength(metadata), + ValidationError, + "Global", + ); +}); + +Deno.test("a v0.6 per-dataset transform is still measured against the intrinsic axes", () => { + const metadata = v06MetadataWithNamedSystemTransform(); + metadata.datasets[1].coordinateTransformations = [createScale([2.0, 2.0])]; + + const error = assertThrows( + () => validateScaleLength(metadata), + ValidationError, + "Dataset 1", + ) as ValidationError; + if (error.rule !== SpecRule.ScaleLengthMismatch) { + throw new Error(`expected scale-length-mismatch, got ${error.rule}`); + } +}); From db2ef606bf77fea2407d5deca4c0420d6b30cab6 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 13:06:02 +0200 Subject: [PATCH 06/10] docs(py): say which structural rules restate a schema keyword The module docstring claimed the rules state only what a JSON Schema cannot. Three of them overlap a keyword and are kept deliberately: axis-count restates minItems and maxItems and holds the floor at 2 where the 0.6 schema drops it to 1; uniqueItems compares whole axis objects, so it is weaker than axis-names-unique, which refuses two axes sharing a name and differing elsewhere; and no bundled schema constrains the omero channel color format, which 0.6 types as a bare string. test_rules_against_schemas.py measures each claim, so a schema tightened upstream fails there rather than leaving the text false. --- py/ngff_zarr/structural_validation.py | 35 ++++-- py/test/test_rules_against_schemas.py | 174 ++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 8 deletions(-) create mode 100644 py/test/test_rules_against_schemas.py diff --git a/py/ngff_zarr/structural_validation.py b/py/ngff_zarr/structural_validation.py index a15d1b3b..000e00a8 100644 --- a/py/ngff_zarr/structural_validation.py +++ b/py/ngff_zarr/structural_validation.py @@ -2,17 +2,36 @@ # SPDX-License-Identifier: MIT """Structural validation for OME-Zarr v0.4 image/multiscales metadata. -This module implements the OME-Zarr v0.4 specification MUSTs that a JSON -Schema cannot express, layered conceptually on top of the schema validation -performed by :mod:`ngff_zarr.validate` (which checks the raw attribute dict). -Where schema validation answers "is this shaped like OME-Zarr?", the rules -here answer "does this obey the spec's structural invariants?" -- e.g. axis -counts, axis ordering, coordinate-transformation arity, finest-to-coarsest -dataset ordering, and OMERO channel color format. +This module implements the OME-Zarr specification MUSTs, layered conceptually +on top of the schema validation performed by :mod:`ngff_zarr.validate` (which +checks the raw attribute dict). Where schema validation answers "is this shaped +like OME-Zarr?", the rules here answer "does this obey the spec's structural +invariants?" -- e.g. axis counts, axis ordering, coordinate-transformation +arity, finest-to-coarsest dataset ordering, and OMERO channel color format. + +Most of these MUSTs are beyond what a JSON Schema states: the equality of a +vector's length and the axis count, an ordering across datasets, a consistency +between two documents. Three rules sit differently, and the overlap is +deliberate rather than an oversight: + +* ``axis-count`` restates ``minItems: 2`` and ``maxItems: 5`` on the axes + definition of the 0.4 and 0.5 schemas. It is kept because it is the only + check on an axis count for a caller without the extra, and because at 0.6 the + schema relaxes ``minItems`` to 1 while the rule holds the floor at 2. +* ``axis-names-unique`` looks like the schemas' ``uniqueItems: true`` and is + stronger. ``uniqueItems`` compares whole axis objects, so two axes sharing a + ``name`` but differing in ``unit`` satisfy it and the rule still refuses them. +* ``omero-channel-color-format`` would be a ``pattern``, and no bundled schema + declares one: 0.6 types ``color`` as a bare string. Until upstream constrains + it, this rule is the only check on the format. + +``test_rules_against_schemas.py`` measures each of those three claims, so a +schema tightened upstream fails there rather than leaving this text false. The rules are pure Python (standard library only) and operate on already parsed metadata, so they run without the optional ``[validate]`` extra -(``jsonschema``) installed. One rule is the exception: +(``jsonschema``) installed, which is the second reason a schema-expressible +rule earns its place here. One rule is the exception: :func:`validate_axis_orientation` delegates to :func:`~ngff_zarr.rfc4_validation.validate_rfc4_orientation`, which imports ``jsonschema``, so a document where some axis carries a non-empty diff --git a/py/test/test_rules_against_schemas.py b/py/test/test_rules_against_schemas.py new file mode 100644 index 00000000..a8d16ff5 --- /dev/null +++ b/py/test/test_rules_against_schemas.py @@ -0,0 +1,174 @@ +# SPDX-FileCopyrightText: Copyright (c) Fideus Labs LLC +# SPDX-License-Identifier: MIT +"""Where the structural rules and the bundled JSON Schemas overlap. + +:mod:`ngff_zarr.structural_validation` carries the spec MUSTs no JSON Schema +states. Three of its rules do restate, or look like they restate, something a +schema declares, and the module docstring says why each is kept. These tests +measure that relationship instead of trusting the prose: if upstream tightens a +schema, or the bounds in a rule drift from it, the claim fails here rather than +quietly becoming false. +""" + +import json + +import pytest +from ngff_zarr.structural_validation import ( + SpecRule, + ValidationError, + validate_axis_names_unique, + validate_structural, +) +from ngff_zarr.v04.zarr_metadata import Axis, Dataset, Metadata, Scale +from ngff_zarr.validate import _schemas_dir + +jsonschema = pytest.importorskip("jsonschema") + +#: The versions whose schemas keep the axes definition inside ``image.schema``. +FLAT_VERSIONS = ["0.4", "0.5"] + + +def _axes_constraints(version: str) -> dict: + """The keywords the bundled schema puts on the axes array.""" + image = json.loads(_schemas_dir(version).joinpath("image.schema").read_text()) + return image["$defs"]["axes"] + + +def _validate(document: dict, version: str = "0.4") -> list: + from jsonschema import Draft202012Validator + from ngff_zarr.validate import _schema_registry, load_schema + + validator = Draft202012Validator( + load_schema(version=version), registry=_schema_registry(version) + ) + return list(validator.iter_errors(document)) + + +def _image_document(axes: list[dict], version: str = "0.4") -> dict: + return { + "multiscales": [ + { + "version": version, + "name": "image", + "axes": axes, + "datasets": [ + { + "path": "0", + "coordinateTransformations": [ + {"type": "scale", "scale": [1.0] * len(axes)} + ], + } + ], + } + ] + } + + +@pytest.mark.parametrize("version", FLAT_VERSIONS) +def test_axis_count_restates_the_schema_bounds_at_0_4_and_0_5(version): + """The rule's 2..5 is the schema's own ``minItems``/``maxItems``.""" + constraints = _axes_constraints(version) + + assert constraints["minItems"] == 2 + assert constraints["maxItems"] == 5 + + +def test_axis_count_is_stricter_than_the_0_6_schema_at_the_low_end(): + """0.6 relaxes the floor to one axis; the rule holds it at two.""" + axes = json.loads(_schemas_dir("0.6").joinpath("axes.schema").read_text()) + + assert axes["minItems"] == 1 + + +@pytest.mark.parametrize("version", FLAT_VERSIONS) +def test_the_axes_array_carries_unique_items(version): + assert _axes_constraints(version)["uniqueItems"] is True + + +def test_unique_items_does_not_catch_two_axes_sharing_a_name(): + """``uniqueItems`` compares whole objects, so a shared name gets through.""" + axes = [ + {"name": "y", "type": "space", "unit": "micrometer"}, + {"name": "y", "type": "space", "unit": "millimeter"}, + ] + + assert _validate(_image_document(axes)) == [] + + +def test_axis_names_unique_refuses_what_unique_items_accepts(): + """The same document the schema accepts is refused by the rule.""" + metadata = Metadata( + axes=[ + Axis(name="y", type="space", unit="micrometer"), + Axis(name="y", type="space", unit="millimeter"), + ], + datasets=[Dataset(path="0", coordinateTransformations=[Scale([1.0, 1.0])])], + coordinateTransformations=None, + ) + + with pytest.raises(ValidationError) as excinfo: + validate_axis_names_unique(metadata) + assert excinfo.value.rule == SpecRule.AXIS_NAMES_UNIQUE + + +def test_a_channel_and_a_space_axis_may_not_share_a_name(): + """The case no schema and no other rule covers, at any version. + + Every other axis rule passes: one channel axis, the space names are the + canonical ``(y, x)`` suffix, and the type order is channel before space. + """ + axes = [ + {"name": "y", "type": "channel"}, + {"name": "y", "type": "space", "unit": "micrometer"}, + {"name": "x", "type": "space", "unit": "micrometer"}, + ] + assert _validate(_image_document(axes), "0.4") == [] + + metadata = Metadata( + axes=[ + Axis(name="y", type="channel"), + Axis(name="y", type="space", unit="micrometer"), + Axis(name="x", type="space", unit="micrometer"), + ], + datasets=[ + Dataset(path="0", coordinateTransformations=[Scale([1.0, 1.0, 1.0])]) + ], + coordinateTransformations=None, + ) + + with pytest.raises(ValidationError) as excinfo: + validate_structural(metadata) + assert excinfo.value.rule == SpecRule.AXIS_NAMES_UNIQUE + assert excinfo.value.location == "multiscales[0].axes[1]" + + +def _omero_channel_properties(version: str) -> dict: + """The channel object the bundled schema declares, wherever it lives.""" + image = json.loads(_schemas_dir(version).joinpath("image.schema").read_text()) + + found = [] + + def walk(node): + if isinstance(node, dict): + channels = node.get("channels") + if isinstance(channels, dict) and isinstance(channels.get("items"), dict): + found.append(channels["items"].get("properties", {})) + for value in node.values(): + walk(value) + elif isinstance(node, list): + for value in node: + walk(value) + + walk(image) + assert found, f"no omero channels definition in the {version} image schema" + return found[0] + + +@pytest.mark.parametrize("version", ["0.4", "0.5", "0.6"]) +def test_no_schema_constrains_the_omero_channel_color_format(version): + """`omero-channel-color-format` has no schema counterpart to defer to.""" + color = _omero_channel_properties(version).get("color") + + assert color is not None, f"the {version} schema declares no channel color" + assert "pattern" not in color, color + assert "enum" not in color, color From 58853b0957d40b631f87a2a55dc416c8a3c5b3a0 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 16:20:20 +0200 Subject: [PATCH 07/10] fix(mcp): validate against the version the store declares validate_structural was called without the version the reader detected. Several rules are gated on it, so a 0.6 array coordinate system and an RFC-3 axis model were both measured against the v0.4 caps and reported as failures the spec does not state. A version ngff-zarr bundles no schema tree for raised a ValueError that the broad handler recorded as a schema failure, so a store at such a version was declared invalid on the strength of a check that never ran. It is reported as a skipped pass, like the missing [validate] extra. --- mcp/ngff_zarr_mcp/tools.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/mcp/ngff_zarr_mcp/tools.py b/mcp/ngff_zarr_mcp/tools.py index a9afc4af..f8b60029 100644 --- a/mcp/ngff_zarr_mcp/tools.py +++ b/mcp/ngff_zarr_mcp/tools.py @@ -390,14 +390,22 @@ async def validate_ome_zarr(store_path: str) -> ValidationResult: validate_ngff(root_attrs, version=version or "0.4") except ImportError: warnings.append(_SCHEMA_PASS_SKIPPED) + except ValueError as no_schema: + # A version ngff-zarr bundles no schema tree for. That says + # nothing about the document, so it is a pass that did not + # run rather than a failure. + warnings.append(f"Schema validation did not run: {no_schema}") except Exception as validation_error: errors.append(f"Schema validation failed: {validation_error}") # The structural rules carry the spec MUSTs no JSON Schema states, # among them the finest-to-coarsest dataset order. They read the - # parsed model, so they run on the multiscales loaded above. + # parsed model, so they run on the multiscales loaded above. The + # detected version is passed on: several rules are gated on it, and + # without it a 0.6 array coordinate system or an RFC-3 axis model is + # measured against the v0.4 caps and reported as a false failure. try: - validate_structural(multiscales.metadata) + validate_structural(multiscales.metadata, version=version) except ValidationError as structural_error: errors.append(f"Structural validation failed: {structural_error}") except ImportError: From c5254d01f381d69c233d5bc662f64841044903e1 Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 17:49:02 +0200 Subject: [PATCH 08/10] fix(mcp,ts): satisfy the type check and the TypeScript formatter mypy refused two things the CI caught and the local suites do not run: the import fallback rebinds validate_ngff to None, which needs the name annotated as optional, and validate_structural is typed against the v0.4 model while the reader hands back whichever version's model it built. deno fmt splits an import list in the new v0.6 transform test. --- mcp/ngff_zarr_mcp/tools.py | 9 +++++++-- .../structural_validation_v06_global_transform_test.ts | 2 +- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/mcp/ngff_zarr_mcp/tools.py b/mcp/ngff_zarr_mcp/tools.py index f8b60029..da2eef5a 100644 --- a/mcp/ngff_zarr_mcp/tools.py +++ b/mcp/ngff_zarr_mcp/tools.py @@ -23,7 +23,9 @@ try: from ngff_zarr import validate as validate_ngff except ImportError: - validate_ngff = None + # Rebinding the name to None is what the call site tests for; mypy sees the + # function type from the import above. + validate_ngff = None # type: ignore[assignment] from .models import ( @@ -405,7 +407,10 @@ async def validate_ome_zarr(store_path: str) -> ValidationResult: # without it a 0.6 array coordinate system or an RFC-3 axis model is # measured against the v0.4 caps and reported as a false failure. try: - validate_structural(multiscales.metadata, version=version) + validate_structural( + multiscales.metadata, # type: ignore[arg-type] + version=version, + ) except ValidationError as structural_error: errors.append(f"Structural validation failed: {structural_error}") except ImportError: diff --git a/ts/test/structural_validation_v06_global_transform_test.ts b/ts/test/structural_validation_v06_global_transform_test.ts index 9b00fc19..ed3db6b4 100644 --- a/ts/test/structural_validation_v06_global_transform_test.ts +++ b/ts/test/structural_validation_v06_global_transform_test.ts @@ -26,8 +26,8 @@ import { ValidationError, } from "../src/utils/structural_validation.ts"; import { - createScale, type CoordinateSystem, + createScale, type Metadata, } from "../src/types/zarr_metadata.ts"; From 7445667a648927502145f0f5156193508fba2d3c Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Tue, 25 Aug 2026 20:34:42 +0200 Subject: [PATCH 09/10] fix(py): keep active only when it is a boolean, and validate by version bool("false") and bool(1) are True, so coercing the omero channel active flag made the two ports read the same document differently: the TypeScript parser keeps the field only when it is a boolean. Python does the same now. The v0.6 structural pass ran without the version the reader detected. A valid v0.6 array coordinate system carries no space axes, so the v0.4 branch of the axis rules refused it. --- py/ngff_zarr/parse_metadata.py | 5 ++++- py/ngff_zarr/v06/zarr_metadata.py | 6 +++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/py/ngff_zarr/parse_metadata.py b/py/ngff_zarr/parse_metadata.py index a433937a..94cf36c0 100644 --- a/py/ngff_zarr/parse_metadata.py +++ b/py/ngff_zarr/parse_metadata.py @@ -89,7 +89,10 @@ def _parse_omero(omero_data: Union[dict, None]) -> Omero | None: color=None if color is None else str(color), window=_parse_window(channel.get("window")), label=None if label is None else str(label), - active=None if active is None else bool(active), + # Kept only when it is a boolean, as the TypeScript parser + # does: bool("false") and bool(1) are True, so coercing would + # make the two ports read the same document differently. + active=active if isinstance(active, bool) else None, ) ) diff --git a/py/ngff_zarr/v06/zarr_metadata.py b/py/ngff_zarr/v06/zarr_metadata.py index dd8b5329..95ffbe06 100644 --- a/py/ngff_zarr/v06/zarr_metadata.py +++ b/py/ngff_zarr/v06/zarr_metadata.py @@ -938,7 +938,11 @@ def _from_zarr_attrs( validate_structural, ) - validate_structural(metadata, ValidateOptions(level=ValidationLevel.STRICT)) + validate_structural( + metadata, + ValidateOptions(level=ValidationLevel.STRICT), + version=declared_version, + ) return metadata, images From 1973ffd446cf7143e92f48ebd9a034ebd0ab072c Mon Sep 17 00:00:00 2001 From: Valentin Boussot Date: Wed, 26 Aug 2026 11:51:21 +0200 Subject: [PATCH 10/10] docs: pass the version in the direct-validation example The paragraph above says omitting version holds every store to the v0.4 axis caps, and the example that follows omits it. A reader copying it measures v0.6 and RFC-3 metadata against caps their version lifts. --- docs/validation/api.md | 6 ++++-- py/ngff_zarr/rfc4_validation.py | 20 ++++++-------------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/docs/validation/api.md b/docs/validation/api.md index dfe7e50a..848900af 100644 --- a/docs/validation/api.md +++ b/docs/validation/api.md @@ -46,8 +46,9 @@ image/multiscales rules. When `options` is `None` it uses `ValidateOptions()`, i.e. `ValidationLevel.STRICT`. `version` is the OME-Zarr version the metadata declares; the axis count, type and order rules are inert for the versions that adopt the RFC-3 axis model (see [[parity]]), so omitting it holds every store to -the v0.4 axis caps. `axis-names-unique` is never inert: RFC-3 states it and no -released schema carries it. A `ValidationError` carries `.rule` (a `SpecRule`), +the v0.4 axis caps. Pass it whenever the metadata is v0.6 or RFC-3, or those +stores are measured against caps their version lifts. `axis-names-unique` is +never inert: RFC-3 states it and no released schema carries it. A `ValidationError` carries `.rule` (a `SpecRule`), `.message` (str), and `.location` (`str | None`); `str(exc)` is `Spec rule [] violated: `. @@ -77,6 +78,7 @@ try: validate_structural( multiscales.metadata, ValidateOptions(level=ValidationLevel.STRICT), # the default + version=multiscales.metadata.version, # pass it: see above ) except ValidationError as exc: print(exc) # "Spec rule [] violated: " diff --git a/py/ngff_zarr/rfc4_validation.py b/py/ngff_zarr/rfc4_validation.py index f08ded6b..5b01c635 100644 --- a/py/ngff_zarr/rfc4_validation.py +++ b/py/ngff_zarr/rfc4_validation.py @@ -186,20 +186,12 @@ def validate_rfc4_orientation(axes: list[dict[str, Any]]) -> None: if not has_orientation: return - # The bundled artifact reports nothing. Its root is ``{"type": "object", - # "additionalProperties": true}`` with every definition parked in ``$defs`` - # and no ``$ref`` reaching them, so any JSON object satisfies it: an axis - # list of strings, an ``axes`` value that is not a list at all, and an - # orientation whose ``type`` and ``value`` are nonsense all pass. Upstream - # ome/ngff#585 and #586 describe the same two defects, the missing entry - # point and the unconstrained ``AnatomicalOrientation``. - # - # Every RFC 4 rule that fires is therefore the hand-written Python above, - # and none of it is redundant with this call. That is what - # ``test_the_bundled_rfc4_schema_checks_nothing`` pins, so the checks above - # cannot be deleted in favour of a schema pass that does not run. The call - # stays because it starts reporting the moment upstream gives the artifact - # a root entry point. + # The bundled artifact reports nothing: its root is an open object with + # every definition parked in ``$defs`` and no ``$ref`` reaching them, so any + # JSON object satisfies it (upstream ome/ngff#585 and #586). The RFC 4 rules + # that fire are the hand-written Python above, which + # ``test_the_bundled_rfc4_schema_checks_nothing`` pins. The call stays: it + # starts reporting the moment upstream gives the artifact an entry point. schema = load_rfc4_orientation_schema() registry = Registry().with_resource( "https://w3id.org/ome/ngff", resource=Resource.from_contents(schema)