diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b731af..9c0d95c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Per-scene objective-lens extraction for Leica LIF conversions. When a LIF + scene's XML carries objective attributes (on + `` and/or `` elements), the audit + record now surfaces them under + `attrs.zarrmony.per_scene[i].objective` with any subset of the keys + `nominal_magnification`, `numerical_aperture`, `immersion`, `model`, + `working_distance_um`. Missing individual fields are omitted from the dict + (never `null` / `0`); scenes with no objective info at all omit the + `objective` key entirely rather than persist an empty dict. (#52) +- Per-scene `OME/METADATA.ome.xml` now emits a top-level + `` populated from the same LIF + extraction, plus per-image `` and `` + references so downstream tooling (napari, OMERO, `ome_types.from_xml`) + reads the objective as standards-compliant OME metadata. Non-LIF readers + and LIF scenes with no objective info emit the pre-#52 no-instrument + shape unchanged. Both per-scene and per-tile (`lif_mosaic="per-tile"`) + write paths participate. (#52) + +### Changed + +- `AUDIT_SCHEMA_VERSION` bumped from `6` → `7` to signal the new optional + `per_scene[i].objective` sub-dict. Reading old stores is unaffected (the + field is additive); consumers pinned to schema `6` should widen their pin. + (#52) + ## [0.8.0] - 2026-07-10 ### Added diff --git a/src/zarrmony/api.py b/src/zarrmony/api.py index eafbc8c..21edbd2 100644 --- a/src/zarrmony/api.py +++ b/src/zarrmony/api.py @@ -43,13 +43,16 @@ select_auto_stitch_cascade, stage_overlap_discrepancy, ) +from zarrmony.metadata.objective import extract_objective from zarrmony.naming import resolve_scene_dirnames, sanitize_scene_name from zarrmony.readers.default import derive_bioio_distribution from zarrmony.readers.plugin import ReaderPlugin, get_reader from zarrmony.writers.bf2raw import write_bf2raw_wrapper from zarrmony.writers.ome_xml import ( + attach_objective_to_image, attach_stage_position_plane, build_combined_ome_xml, + build_instrument_from_objective, build_ome_xml_for_scene, ) from zarrmony.writers.per_scene import write_per_scene_metadata @@ -232,16 +235,33 @@ def _scene_channel_count(reader: Any) -> int: return int(xarr.sizes["C"]) if "C" in xarr.dims else 1 -def _lif_scene_channels(reader: Any) -> tuple[list[dict] | None, list[Channel] | None]: +def _lif_scene_channels( + reader: Any, +) -> tuple[list[dict] | None, list[Channel] | None, dict | None]: """The LIF-vs-other decision, in one place. - Returns ``(extracted, omero_channels)`` when ``reader`` is a LIF reader whose - current scene yields channel identities AND that identity count matches the - scene's C size; otherwise ``(None, None)``. ``extracted`` is the raw - per-channel identity dicts (handed to :func:`_lif_ome_image` once scene sizes - are known); ``omero_channels`` is the display label/color list for - :func:`write_scene`. Both projections come from the same vetted extraction, - so omero and the OME-XML never disagree. + Returns ``(extracted, omero_channels, objective)``: + + * ``extracted`` / ``omero_channels`` — per-channel identity dicts + the + display label/color list. Both are populated only when ``reader`` is a + LIF reader whose current scene yields channel identities AND that + identity count matches the scene's C size; otherwise both are ``None``. + Both projections come from the same vetted extraction, so omero and the + OME-XML never disagree. + * ``objective`` — the LIF objective-lens dict (issue #52), independently + derived from the same scene XML. Populated whenever + :func:`~zarrmony.metadata.objective.extract_objective` returns a + non-empty dict; ``None`` for a non-LIF reader, missing scene XML, or a + scene with no objective info at all. It carries only the keys the LIF + actually surfaced — missing individual fields (magnification, NA, + immersion, model, working distance) are omitted rather than nulled. The + audit records it under ``per_scene[i].objective`` and the per-scene + writer projects it into a top-level ````. + + Objective extraction is deliberately independent of the channel count + check: a LIF scene with a garbled channel list (mismatched SizeC) still + has a valid objective, and the audit shouldn't drop it just because we + also can't determine the fluorophores. Both extractors are fail-closed. Locating the scene XML is two-tier (see :func:`find_scene_xml`): @@ -251,30 +271,35 @@ def _lif_scene_channels(reader: Any) -> tuple[list[dict] | None, list[Channel] | 2. the document-order ```` locator (``.//Image[current_scene_index]``, per bioio-lif PR #52) — the fallback that makes confocal scenes work. - The count check matters: the omero block and OME-XML ``Pixels/@SizeC`` must - describe the channels the array actually has. If extraction and data - disagree in EITHER direction — too few or too many identities (corruption, - partial metadata) — we decline the projection rather than mislabel the image. - - Fail-safe: any failure — not a LIF reader, no/garbage scene XML, an empty or - count-mismatched extraction, or any unexpected reader-surface error — returns - ``(None, None)`` so callers fall back cleanly to the existing name-based - path. Metadata never crashes a conversion. + The count check matters for channels: the omero block and OME-XML + ``Pixels/@SizeC`` must describe the channels the array actually has. If + extraction and data disagree in EITHER direction — too few or too many + identities (corruption, partial metadata) — we decline the *channel* + projection rather than mislabel the image. Objective extraction has no + such coupling. + + Fail-safe: any failure — not a LIF reader, no/garbage scene XML, or any + unexpected reader-surface error — returns ``(None, None, None)`` so + callers fall back cleanly to the existing name-based path. Metadata never + crashes a conversion. """ try: scene_xml = find_scene_xml(reader) if scene_xml is None: - return None, None + return None, None, None extracted = extract_channels(scene_xml) + objective = extract_objective(scene_xml) if not extracted or len(extracted) != _scene_channel_count(reader): - return None, None + # Channel extraction fell through the count check, but the + # objective is decoupled from SizeC — still surface it. + return None, None, objective omero_channels = [ Channel(label=o["label"], color=o["color"]) for o in channels_to_omero(extracted) ] - return extracted, omero_channels + return extracted, omero_channels, objective except Exception: # noqa: BLE001 — never break a conversion over metadata - return None, None + return None, None, None def _lif_ome_image( @@ -300,6 +325,34 @@ def _lif_ome_image( return None +def _instrument_for_objective( + objective: dict | None, image: Image | None, scene_index: int +) -> Any | None: + """Build the per-image ```` from ``objective`` and attach refs. + + Returns the ``Instrument`` (for the caller to hand to + :func:`build_ome_xml_for_scene`) when ``objective`` is non-empty and + ``image`` is present; otherwise ``None`` — the pre-#52 shape. Scoped IDs + include ``scene_index`` so a bf2raw / plate consumer parsing a combined + OME-XML sees distinct instruments per image. + + Never raises: objective is metadata, and metadata never breaks a + conversion. + """ + if not objective or image is None: + return None + try: + instrument = build_instrument_from_objective( + objective, + instrument_id=f"Instrument:{scene_index}", + objective_id=f"Objective:{scene_index}:0", + ) + attach_objective_to_image(image, instrument=instrument) + return instrument + except Exception: # noqa: BLE001 — never break a conversion over metadata + return None + + def _source_xml_filename(input_path: str | Path) -> str: ext = Path(str(input_path)).suffix.lstrip(".").lower() return f"raw.{ext}.xml" if ext else "raw.xml" @@ -783,8 +836,10 @@ def _convert_per_scene( # LIF-vs-other decision in one place: for a LIF scene with extractable # channel identities, drive both omero (label/color) and the canonical # OME-XML s from that identity; otherwise fall through to the - # existing name-based path. (None, None) for everything non-LIF. - lif_extracted, lif_omero_channels = _lif_scene_channels(reader) + # existing name-based path. (None, None, None) for everything non-LIF. + # ``lif_objective`` is decoupled from the channel-count check so a + # scene with a garbled channel list still surfaces its objective. + lif_extracted, lif_omero_channels, lif_objective = _lif_scene_channels(reader) channels = ( lif_omero_channels if lif_omero_channels is not None @@ -861,7 +916,15 @@ def _convert_per_scene( ) ome_image = _stub_image(scene_index, scene_name, scene_record) - ome_xml = build_ome_xml_for_scene(ome_image) + # LIF objective (issue #52): audit gets the raw dict; OME-XML gets a + # top-level plus a per-image + # reference. Both are omitted for non-LIF readers + # and for LIF scenes that surfaced no objective info at all. + if lif_objective: + scene_record["objective"] = lif_objective + instrument = _instrument_for_objective(lif_objective, ome_image, scene_index) + + ome_xml = build_ome_xml_for_scene(ome_image, instrument) write_per_scene_metadata( store_path, ome_xml=ome_xml, @@ -996,8 +1059,10 @@ def _convert_per_tile_scene( # Channel projection: as in per-scene mode, the LIF extracted identity wins; # otherwise fall through to the name-based path. Same channels for every - # tile of the same scene. - lif_extracted, lif_omero_channels = _lif_scene_channels(reader) + # tile of the same scene. Objective is likewise scene-scoped (one + # objective per scene, shared by all tiles) and re-attached to each + # tile's OME-XML below. + lif_extracted, lif_omero_channels, lif_objective = _lif_scene_channels(reader) channels = ( lif_omero_channels if lif_omero_channels is not None @@ -1078,7 +1143,14 @@ def _convert_per_tile_scene( position_z_um=pos_z_um, ) - ome_xml = build_ome_xml_for_scene(ome_image) + # LIF objective (issue #52): the scene's objective is shared across + # all tiles; every tile store gets its own audit copy and its own + # so any tile store remains fully self-describing. + if lif_objective: + scene_record["objective"] = lif_objective + instrument = _instrument_for_objective(lif_objective, ome_image, scene_index) + + ome_xml = build_ome_xml_for_scene(ome_image, instrument) write_per_scene_metadata( store_path, ome_xml=ome_xml, diff --git a/src/zarrmony/audit.py b/src/zarrmony/audit.py index 4628edb..8a6fb37 100644 --- a/src/zarrmony/audit.py +++ b/src/zarrmony/audit.py @@ -4,8 +4,10 @@ recording: zarrmony version, the winning reader plugin (name, distribution, source, version, match score), the input file's path / size / mtime / optional SHA256, the conversion config the user passed, started/finished timestamps, -per-scene records returned by ``write_scene``, and any extractor-failure -warnings. +per-scene records returned by ``write_scene`` (which for LIF conversions may +carry an ``objective`` sub-dict with ``nominal_magnification`` / +``numerical_aperture`` / ``immersion`` / ``model`` / ``working_distance_um``), +and any extractor-failure warnings. Stored as a top-level ``attrs.zarrmony`` (not under ``attrs.ome``) to keep the spec-defined namespace clean. ``audit_schema_version`` is bumped whenever this @@ -25,7 +27,12 @@ from zarrmony._storage import format_bytes, open_root_group, size_on_disk from zarrmony.readers.plugin import ReaderPlugin -AUDIT_SCHEMA_VERSION = 6 +# 7: adds optional ``per_scene[i].objective`` (nominal_magnification / +# numerical_aperture / immersion / model / working_distance_um) from the +# LIF objective-lens extractor. Missing fields are omitted; scenes with no +# objective info omit the ``objective`` key entirely. Purely additive: +# consumers pinned to 6 can widen their pin. (#52) +AUDIT_SCHEMA_VERSION = 7 def _file_forensics(path: str | Path, *, checksum: bool = False) -> dict[str, Any]: diff --git a/src/zarrmony/metadata/objective.py b/src/zarrmony/metadata/objective.py new file mode 100644 index 0000000..0f11adc --- /dev/null +++ b/src/zarrmony/metadata/objective.py @@ -0,0 +1,250 @@ +"""Pure, stdlib-only per-scene objective-lens extractor for Leica LIF scene XML. + +A Leica LIF scene records the acquisition objective in two overlapping shapes: + +* every ```` (the sequential-settings blocks that + :mod:`zarrmony.metadata.lif_channels` already walks for channel identity) carries + the objective as *attributes* — ``ObjectiveName``, ``NumericalAperture``, + ``Immersion``, and either ``ObjectiveMag`` / ``NominalMagnification`` when + present, or an ``NNx`` embedded in ``ObjectiveName`` (``"HC PL APO CS2 20x/0.75 DRY"``). +* some LIF exports also include an ```` element carrying the same + fields; walk it and merge so either shape yields the same audit dict. + +:func:`extract_objective` returns one dict per scene with only the keys the LIF +actually surfaced — missing fields are omitted (never ``None`` / ``0``), and a +scene with no objective info at all yields ``None`` (rather than an empty dict). +This is the shape the audit persists under ``per_scene[i].objective`` (issue #52) +and the shape :mod:`zarrmony.writers.ome_xml` projects into a top-level +```` + per-image ````. + +The extractor is *dependency-free* (stdlib only) for the same reason +:mod:`lif_channels` is: it is meant to lift into ``bioio-lif`` unchanged. + +Hardening: fail-closed like the sibling extractors. Oversized input, DTDs / +entity declarations (billion-laughs / XXE), external entities, and any +malformed XML all yield ``None`` — never an exception, never entity expansion, +never a hang. Metadata never crashes a conversion. +""" + +from __future__ import annotations + +import math +import re +import xml.etree.ElementTree as ET + +# Mirrors the caps used by lif_channels / lif_tiles. A confocal scene blob is +# a few hundred KB; 32 MiB is generous headroom while still bounding parse work. +_MAX_BYTES = 32 * 1024 * 1024 + +# Any DOCTYPE / entity declaration in a LIF scene blob is malformed or hostile. +# Reject textually before parsing — stdlib ``ElementTree`` *does* expand +# internal entities. +_DOCTYPE_OR_ENTITY = re.compile(r" None: + self._builder = ET.TreeBuilder() + + def entity_decl(self, *_args, **_kwargs): # pragma: no cover - defensive + raise ValueError("entity declarations are not permitted") + + def unparsed_entity_decl(self, *_args, **_kwargs): # pragma: no cover + raise ValueError("entity declarations are not permitted") + + def start_doctype_decl(self, *_args, **_kwargs): # pragma: no cover + raise ValueError("DOCTYPE is not permitted") + + def start(self, tag, attrib): + return self._builder.start(tag, attrib) + + def end(self, tag): + return self._builder.end(tag) + + def data(self, text): + return self._builder.data(text) + + def close(self): + return self._builder.close() + + +def _safe_parse(scene_xml: str) -> ET.Element | None: + """Parse ``scene_xml`` into a root element, fail-closed (never raises).""" + if not isinstance(scene_xml, str) or not scene_xml: + return None + if len(scene_xml.encode("utf-8", "ignore")) > _MAX_BYTES: + return None + if _DOCTYPE_OR_ENTITY.search(scene_xml): + return None + try: + parser = ET.XMLParser(target=_EntityRejectingTarget()) + parser.feed(scene_xml) + return parser.close() + except Exception: + return None + + +def _to_number(text: str | None) -> int | float | None: + """Parse a numeric string, returning int when integral, else float, else None.""" + if text is None: + return None + try: + value = float(text) + except (TypeError, ValueError): + return None + if not math.isfinite(value): + return None + return int(value) if value.is_integer() else value + + +def _clean_model(raw: str | None) -> str | None: + """Collapse LIF's whitespace-padded objective name into a canonical string. + + ``"HC PL APO CS2 20x/0.75 DRY "`` → ``"HC PL APO CS2 20x/0.75 DRY"``. + Empty / whitespace-only inputs degrade to ``None`` so the caller omits the + ``model`` key rather than persisting a bogus placeholder. + """ + if raw is None: + return None + collapsed = " ".join(raw.split()).strip() + return collapsed or None + + +def _map_immersion(raw: str | None) -> str | None: + """LIF ``Immersion`` attribute → OME ``Objective_Immersion`` enum value. + + Unknown-but-present strings map to ``"Other"`` (never dropped: "we saw an + immersion we don't recognise" is different from "no immersion metadata"). + A missing / empty attribute yields ``None`` so the caller omits the key. + """ + if raw is None: + return None + key = str(raw).strip().upper() + if not key: + return None + return _LIF_IMMERSION_TO_OME.get(key, "Other") + + +def _parse_name_magnification(name: str | None) -> int | float | None: + """Extract the ``NNx`` magnification embedded in an objective name, or ``None``. + + ``"HC PL APO CS2 20x/0.75 DRY"`` → ``20``; ``"63x/1.40 OIL"`` → ``63``. + Returns ``None`` when the pattern is absent so the caller keeps searching + for a magnification via other attributes. + """ + if not name: + return None + match = _OBJECTIVE_NAME_MAG.search(name) + if match is None: + return None + return _to_number(match.group(1)) + + +def _fill_from_attrs(dest: dict, attrib: dict) -> None: + """Merge one element's attributes into ``dest``, first-writer-wins per key. + + ``nominal_magnification`` prefers an explicit ``ObjectiveMag`` / + ``NominalMagnification`` attribute; if neither is present it falls back to + parsing an ``NNx`` from ``ObjectiveName``. The bare ``Magnification`` + attribute on ``ATLConfocalSettingDefinition`` is *total* zoom (objective + magnification × zoom factor) and is deliberately NOT read — recording ``21`` + instead of ``20`` for a ``20x`` objective would silently corrupt the audit. + + ``working_distance_um`` is taken verbatim from the ``WorkingDistance`` + attribute. LIF's stored unit for this field is not consistently documented + (some exports millimeters, others micrometers); recording the raw value + preserves fidelity — consumers that know their instrument can convert. + """ + if "nominal_magnification" not in dest: + for key in ("ObjectiveMag", "NominalMagnification"): + value = _to_number(attrib.get(key)) + if value is not None: + dest["nominal_magnification"] = value + break + if "nominal_magnification" not in dest: + value = _parse_name_magnification(attrib.get("ObjectiveName")) + if value is not None: + dest["nominal_magnification"] = value + + if "numerical_aperture" not in dest: + value = _to_number(attrib.get("NumericalAperture")) + if value is not None: + dest["numerical_aperture"] = value + + if "immersion" not in dest: + mapped = _map_immersion(attrib.get("Immersion")) + if mapped is not None: + dest["immersion"] = mapped + + if "model" not in dest: + model = _clean_model(attrib.get("ObjectiveName") or attrib.get("Model")) + if model is not None: + dest["model"] = model + + if "working_distance_um" not in dest: + value = _to_number(attrib.get("WorkingDistance")) + if value is not None: + dest["working_distance_um"] = value + + +def extract_objective(scene_xml: str) -> dict | None: + """Extract the acquisition objective from one Leica LIF scene XML string. + + Returns a dict with any subset of the keys ``nominal_magnification``, + ``numerical_aperture``, ``immersion``, ``model``, ``working_distance_um`` — + only the keys the LIF actually surfaced. Missing individual fields are + *omitted* (never ``None`` / ``0``); a scene with no objective info at all + yields ``None`` rather than an empty dict, so the audit either records the + ``objective`` key with real content or omits it entirely. + + Both ```` and ```` elements are + walked and merged (first-writer-wins per key): different LIF exports carry + the same fields on either element, and either shape yields the same audit + dict. + + Fail-closed: any parse failure, unsafe input, or missing structure yields + ``None``. Metadata never crashes a conversion. + """ + root = _safe_parse(scene_xml) + if root is None: + return None + try: + result: dict = {} + for atl in root.iter("ATLConfocalSettingDefinition"): + _fill_from_attrs(result, atl.attrib) + for obj in root.iter("Objective"): + _fill_from_attrs(result, obj.attrib) + return result or None + except Exception: + # Any unexpected structural surprise stays fail-closed. + return None diff --git a/src/zarrmony/writers/ome_xml.py b/src/zarrmony/writers/ome_xml.py index 3500247..930dbf1 100644 --- a/src/zarrmony/writers/ome_xml.py +++ b/src/zarrmony/writers/ome_xml.py @@ -5,12 +5,27 @@ Per the OME-Zarr spec, each Image's Pixels MUST use ```` (not BinData / TiffData / BinaryOnly) because the pixel data lives in the sibling Zarr arrays. + +The ``build_*`` helpers also carry an optional ``instruments`` list — the +top-level ```` element(s) each ```` may reference via its +```` + ```` children. See +:func:`build_instrument_from_objective` for the LIF-shaped projection consumed +by the per-scene writer (issue #52). """ from collections.abc import Iterable from ome_types import OME -from ome_types.model import Image, MetadataOnly, Plane, UnitsLength +from ome_types.model import ( + Image, + Instrument, + InstrumentRef, + MetadataOnly, + Objective, + ObjectiveSettings, + Plane, + UnitsLength, +) def normalize_image_for_metadata_only(image: Image) -> Image: @@ -58,17 +73,86 @@ def attach_stage_position_plane( return image -def build_combined_ome_xml(images: Iterable[Image]) -> str: +def build_combined_ome_xml( + images: Iterable[Image], + instruments: Iterable[Instrument] = (), +) -> str: """Combine per-scene Image elements into a single OME-XML document. The order of images in the returned XML matches the iteration order; that same order MUST be reflected in the bf2raw ``OME/series`` attribute. + + ``instruments`` — optional top-level ```` elements referenced + by the images via their ```` + ````. + Emitted before ```` per the OME schema's declared order. Default + empty tuple preserves the pre-#52 no-instrument shape. """ images_list = [normalize_image_for_metadata_only(img) for img in images] - ome = OME(images=images_list) + ome = OME(instruments=list(instruments), images=images_list) return ome.to_xml() -def build_ome_xml_for_scene(image: Image) -> str: - """Build a single-Image OME-XML document for one per-scene store.""" - return build_combined_ome_xml([image]) +def build_ome_xml_for_scene(image: Image, instrument: Instrument | None = None) -> str: + """Build a single-Image OME-XML document for one per-scene store. + + ``instrument`` — optional top-level ```` referenced by + ``image.instrument_ref`` / ``image.objective_settings``. When provided it + is emitted alongside the image; when ``None`` the output shape is + unchanged (pre-#52 behaviour). + """ + return build_combined_ome_xml( + [image], [instrument] if instrument is not None else [] + ) + + +def build_instrument_from_objective( + objective: dict, + *, + instrument_id: str = "Instrument:0", + objective_id: str = "Objective:0", +) -> Instrument: + """Project a LIF-extracted objective dict into an OME ````. + + Consumes the shape :func:`zarrmony.metadata.objective.extract_objective` + returns — one dict with any subset of ``nominal_magnification``, + ``numerical_aperture``, ``immersion``, ``model``, ``working_distance_um``. + Only present keys are set on the OME ```` element; the OME + element itself always carries its ``ID`` attribute (required by the schema + for cross-references from ````). + + Working distance is emitted with an explicit ``µm`` unit; the caller must + already have converted whatever LIF's raw value was into micrometers. + """ + fields: dict = {"id": objective_id} + if "model" in objective: + fields["model"] = objective["model"] + if "nominal_magnification" in objective: + fields["nominal_magnification"] = float(objective["nominal_magnification"]) + if "numerical_aperture" in objective: + fields["lens_na"] = float(objective["numerical_aperture"]) + if "immersion" in objective: + fields["immersion"] = objective["immersion"] + if "working_distance_um" in objective: + fields["working_distance"] = float(objective["working_distance_um"]) + fields["working_distance_unit"] = UnitsLength.MICROMETER + return Instrument(id=instrument_id, objectives=[Objective(**fields)]) + + +def attach_objective_to_image( + image: Image, + *, + instrument: Instrument, +) -> Image: + """Stamp ```` + ```` onto ``image``. + + The image gains references to ``instrument`` (top-level) and to its first + (only) objective. Both refs are required for the OME-XML to round-trip: + ```` inside the ```` element, + and ```` (same ID as the objective + inside the instrument) so consumers know *which* objective on that + instrument was actually used. Returns ``image`` for chaining; mutates in + place. + """ + image.instrument_ref = InstrumentRef(id=instrument.id) + image.objective_settings = ObjectiveSettings(id=instrument.objectives[0].id) + return image diff --git a/tests/test_api_lif_wiring.py b/tests/test_api_lif_wiring.py index 2fc384d..52d4c0c 100644 --- a/tests/test_api_lif_wiring.py +++ b/tests/test_api_lif_wiring.py @@ -122,24 +122,34 @@ def xarray_dask_data(self): def test_raising_scene_root_without_metadata_declines(): - assert _lif_scene_channels(_Raising()) == (None, None) + assert _lif_scene_channels(_Raising()) == (None, None, None) def test_non_lif_falls_back(): - assert _lif_scene_channels(_NonLif()) == (None, None) + assert _lif_scene_channels(_NonLif()) == (None, None, None) -def test_count_mismatch_declines(): - assert _lif_scene_channels(_CountMismatch()) == (None, None) +def test_count_mismatch_declines_channels_but_still_surfaces_objective(): + # Objective extraction is decoupled from the SizeC check — a garbled + # channel list must not drop the (still-valid) objective dict. + extracted, omero, objective = _lif_scene_channels(_CountMismatch()) + assert extracted is None and omero is None + assert objective is not None and objective.get("nominal_magnification") == 20 # --- plate fast path ------------------------------------------------------- def test_happy_path_wiring(): - extracted, omero = _lif_scene_channels(_Good()) + extracted, omero, objective = _lif_scene_channels(_Good()) assert extracted is not None and len(extracted) == 7 assert [c.label for c in omero][1] == "ALEXA 594 (590 nm)" + # Objective co-extraction: same scene XML, same call — the fixture's + # ATLConfocalSettingDefinition carries a 20x/0.75 DRY objective. + assert objective is not None + assert objective["nominal_magnification"] == 20 + assert objective["numerical_aperture"] == 0.75 + assert objective["immersion"] == "Air" # LIF "DRY" -> OME Air # --- confocal (scene_root raises -> metadata .//Image locator) ------------- @@ -148,7 +158,7 @@ def test_happy_path_wiring(): def test_confocal_scene_yields_fluorophore_channels(): r = _ConfocalReader() r.current_scene_index = 0 - extracted, omero = _lif_scene_channels(r) + extracted, omero, _ = _lif_scene_channels(r) assert omero is not None, "confocal channel metadata is inert" assert [c.label for c in omero][1] == "ALEXA 594 (590 nm)" assert extracted[1]["excitation_nm"] == 590 @@ -157,16 +167,19 @@ def test_confocal_scene_yields_fluorophore_channels(): def test_confocal_scene_is_located_positionally(): r = _ConfocalReader() r.current_scene_index = 1 - _, omero = _lif_scene_channels(r) + _, omero, _ = _lif_scene_channels(r) assert omero is not None assert [c.label for c in omero][1] == "B_Leica/ALEXA 594 (590 nm)" def test_confocal_count_mismatch_declines(): r = _ConfocalReader(c=2) # XML has 7 channels, array claims 2 - assert _lif_scene_channels(r) == (None, None) + extracted, omero, objective = _lif_scene_channels(r) + assert (extracted, omero) == (None, None) + # Objective is still surfaced — see the plate-path test for the reason. + assert objective is not None def test_confocal_no_metadata_falls_back(): r = _ConfocalReader(have_metadata=False) - assert _lif_scene_channels(r) == (None, None) + assert _lif_scene_channels(r) == (None, None, None) diff --git a/tests/test_integration_ome_tiff.py b/tests/test_integration_ome_tiff.py index d6e1d08..5b90795 100644 --- a/tests/test_integration_ome_tiff.py +++ b/tests/test_integration_ome_tiff.py @@ -43,7 +43,7 @@ def test_per_scene_single_scene_ome_tiff_round_trip(tmp_path: Path) -> None: assert audit["reader_plugin"]["distribution"] == "bioio-ome-tiff" assert audit["reader_plugin"]["source"] == "builtin" assert audit["reader_plugin"]["match_score"] == 0 - assert audit["audit_schema_version"] == 6 + assert audit["audit_schema_version"] == 7 assert audit["input"]["size_bytes"] > 0 assert audit["input"]["size_human"] @@ -148,7 +148,7 @@ def test_bf2raw_single_scene_ome_tiff_round_trip(tmp_path: Path) -> None: assert audit["reader_plugin"]["name"] == "bioio" assert audit["reader_plugin"]["distribution"] == "bioio-ome-tiff" - assert audit["audit_schema_version"] == 6 + assert audit["audit_schema_version"] == 7 assert len(audit["per_scene"]) == 1 with open(out / "zarr.json") as f: diff --git a/tests/test_lif_objective.py b/tests/test_lif_objective.py new file mode 100644 index 0000000..ae3a670 --- /dev/null +++ b/tests/test_lif_objective.py @@ -0,0 +1,240 @@ +"""Tests for the stdlib-only Leica LIF objective-lens extractor. + +Exercises the real captured confocal fixture (attributes on +``ATLConfocalSettingDefinition``), the sibling ```` shape some +LIF exports use, the "complete"/"partial"/"absent" acceptance triple from +issue #52, and the fail-closed hardening shared with +:mod:`zarrmony.metadata.lif_channels`. +""" + +from pathlib import Path + +from zarrmony.metadata.objective import extract_objective + +FIXTURE = Path(__file__).parent / "fixtures" / "lif_confocal_7ch.xml" + + +# --- real captured confocal fixture ---------------------------------------- + + +def test_real_fixture_yields_objective_fields() -> None: + obj = extract_objective(FIXTURE.read_text(encoding="utf-8")) + assert obj is not None + # ObjectiveName in the fixture is "HC PL APO CS2 20x/0.75 DRY "; + # the extractor collapses whitespace and strips the trailing space. + assert obj["model"] == "HC PL APO CS2 20x/0.75 DRY" + # The bare ATLConfocalSettingDefinition Magnification="21" is *total* + # magnification (obj_mag * zoom) and MUST NOT be used; the true objective + # magnification is recovered from the "20x" embedded in the name. + assert obj["nominal_magnification"] == 20 + assert obj["numerical_aperture"] == 0.75 + # LIF "DRY" is Leica shorthand for an air (no-medium) objective. + assert obj["immersion"] == "Air" + + +def test_real_fixture_returns_only_known_keys() -> None: + obj = extract_objective(FIXTURE.read_text(encoding="utf-8")) + # The fixture does not carry WorkingDistance — the key must be omitted + # (never null / 0), per the "missing individual fields" rule in #52. + assert "working_distance_um" not in obj + assert set(obj).issubset( + { + "nominal_magnification", + "numerical_aperture", + "immersion", + "model", + "working_distance_um", + } + ) + + +# --- acceptance triple: complete / partial / absent ------------------------ +# +# Per #52 the writer must handle three cases distinctly: a fully populated +# objective block, a partial one (magnification + NA only), and a scene with +# no objective info at all (``objective`` key OMITTED from the audit, not +# a dict of Nones). Hand-crafted minimal XML fixtures pin each case. + + +def _wrap_atl(attrs: str) -> str: + """Wrap a bare attribute string into a minimal valid LIF-shaped XML doc. + + The extractor walks by tag name (``ATLConfocalSettingDefinition``) so + ancestry beyond a container root is irrelevant to the assertion under test. + """ + return f"{attrs}" + + +def test_complete_objective_populates_every_field() -> None: + xml = _wrap_atl( + '' + ) + obj = extract_objective(xml) + assert obj == { + "nominal_magnification": 63, + "numerical_aperture": 1.4, + "immersion": "Oil", + "model": "HC PL APO CS2 63x/1.40 OIL", + "working_distance_um": 140, + } + + +def test_partial_objective_omits_missing_fields() -> None: + # Magnification + NA only — per #52's "partial" acceptance case. The + # missing model / immersion / working_distance_um keys MUST be absent + # from the dict entirely, not set to None or 0. + xml = _wrap_atl( + '' + ) + obj = extract_objective(xml) + assert obj == {"nominal_magnification": 20, "numerical_aperture": 0.8} + assert "immersion" not in obj + assert "model" not in obj + assert "working_distance_um" not in obj + + +def test_scene_with_no_objective_info_returns_none() -> None: + # A scene XML with no ATLConfocalSettingDefinition and no + # element must yield None (NOT an empty dict) — that's the discriminator + # the audit uses to omit the ``objective`` key from ``per_scene[i]``. + assert extract_objective("") is None + + +# --- alternative source: sibling element ----------------------- + + +def test_objective_element_supplies_fields() -> None: + # Some LIF exports put the same fields on an element beside + # (or instead of) the ATL block. Either shape must yield the same dict. + xml = ( + "" + '' + "" + ) + obj = extract_objective(xml) + assert obj == { + "nominal_magnification": 40, + "numerical_aperture": 1.1, + "immersion": "Water", + "model": "Plan-Apo 40x/1.10 W", + } + + +def test_atl_and_objective_merge_first_writer_wins() -> None: + # When both surfaces carry fields, first-writer-wins per key — the + # ATL block is iterated first, so its ObjectiveName wins over the + # Objective element's Model. Fields only present on the Objective + # element still contribute (the merge is *union*, not exclusion). + xml = ( + "" + '' + '' + "" + ) + obj = extract_objective(xml) + assert obj["model"] == "HC PL APO 63x/1.4 OIL" + assert obj["numerical_aperture"] == 1.4 + assert obj["nominal_magnification"] == 63 # from the Objective element + assert obj["immersion"] == "Oil" + + +# --- immersion mapping (LIF strings → OME enum) ---------------------------- + + +def test_immersion_maps_dry_to_air() -> None: + # LIF "DRY" is not the OME enum value; it's Leica shorthand for air. + xml = _wrap_atl('') + assert extract_objective(xml) == {"immersion": "Air"} + + +def test_immersion_maps_glyc_to_glycerol() -> None: + xml = _wrap_atl('') + assert extract_objective(xml) == {"immersion": "Glycerol"} + + +def test_immersion_unknown_falls_through_to_other() -> None: + # An unrecognised-but-present immersion string is meaningful ("we saw + # an immersion medium we don't recognise") and must map to "Other" + # rather than being dropped — that's a different signal from "no + # immersion metadata at all". + xml = _wrap_atl('') + assert extract_objective(xml) == {"immersion": "Other"} + + +def test_immersion_empty_attribute_is_omitted() -> None: + xml = _wrap_atl('') + assert extract_objective(xml) is None + + +# --- graceful degradation -------------------------------------------------- + + +def test_magnification_from_name_when_attribute_missing() -> None: + # No ObjectiveMag / NominalMagnification, but the name embeds "20x". + xml = _wrap_atl( + '' + ) + obj = extract_objective(xml) + assert obj["nominal_magnification"] == 20 + assert obj["model"] == "HC PL APO 20x/0.75 DRY" + + +def test_bare_magnification_attr_is_ignored() -> None: + # ATL's "Magnification" attribute is total zoom (obj_mag * zoom), NOT the + # objective's nominal magnification. Recording it would silently corrupt + # the audit (e.g. 21 for a 20x objective at 1.05x zoom). + xml = _wrap_atl('') + assert extract_objective(xml) is None + + +def test_non_finite_numeric_values_degrade_to_omission() -> None: + xml = _wrap_atl( + '' + ) + assert extract_objective(xml) is None + + +# --- fail-closed hardening (mirrors lif_channels) -------------------------- + + +def test_billion_laughs_returns_none_fast() -> None: + bomb = '\n\n' + prev = "a" + for i in range(2, 12): + bomb += f' \n' + prev = f"a{i}" + bomb += "]>\n&a11;" + assert extract_objective(bomb) is None + + +def test_external_entity_returns_none() -> None: + xxe = ( + '' + ']>' + "&x;" + ) + assert extract_objective(xxe) is None + + +def test_doctype_is_rejected_even_without_entities() -> None: + assert extract_objective('') is None + + +def test_malformed_xml_returns_none() -> None: + assert extract_objective(" None: + assert extract_objective("") is None + assert extract_objective(None) is None # type: ignore[arg-type] + + +def test_oversized_input_returns_none() -> None: + huge = "" + ("x" * (33 * 1024 * 1024)) + "" + assert extract_objective(huge) is None diff --git a/tests/test_lif_objective_wiring.py b/tests/test_lif_objective_wiring.py new file mode 100644 index 0000000..8786e88 --- /dev/null +++ b/tests/test_lif_objective_wiring.py @@ -0,0 +1,191 @@ +"""End-to-end wiring tests for LIF objective-lens metadata (issue #52). + +Drives ``convert()`` against a FakeReader whose ``metadata`` is a LIF-shaped +XML blob carrying (or deliberately not carrying) an +```` with objective attributes. Asserts, for each +of the three acceptance cases in #52 — complete / partial / absent — both: + +* the on-disk audit at ``attrs.zarrmony.per_scene[0].objective``, and +* the on-disk OME-XML's ```` + + ```` reference on the ````. + +Uses the same ``FakeReader`` + ``patched_reader`` doubles as the rest of the +LIF wiring tests so no real ``bioio_lif`` import is needed. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest +from ome_types import from_xml + +from tests.conftest import FakeReader +from zarrmony import api as api_module +from zarrmony import convert +from zarrmony.readers.plugin import ReaderPlugin + + +def _fake_plugin() -> ReaderPlugin: + return ReaderPlugin( + name="bioio-lif", + match=lambda _p: 100, + open=lambda _p: object(), + distribution="bioio-lif", + source="builtin", + ) + + +@pytest.fixture +def patched_reader(monkeypatch: pytest.MonkeyPatch): + def installer(reader: FakeReader): + monkeypatch.setattr( + api_module, + "get_reader", + lambda _path: (reader, _fake_plugin(), 100), + ) + + return installer + + +def _lif_metadata_with_atl(atl_attrs: str) -> str: + """A LIF-shaped metadata blob with one ```` carrying a single ATL + settings block containing ``atl_attrs`` (which may be empty for the + "no objective info" case). + + The extractor doesn't require the surrounding ChannelDescription / + Sequential blocks — objective attrs live on ATLConfocalSettingDefinition + directly — so we keep the fixture minimal. + """ + atl = f"" if atl_attrs else "" + return ( + "" + "" + f'{atl}' + "" + ) + + +def _reader_with_metadata(metadata: str) -> FakeReader: + return FakeReader( + scenes=["scene0"], + dims="TCYX", + shape=(1, 1, 16, 16), + channel_names=["DAPI"], + raw_xml=metadata, + ) + + +# --- complete objective block ---------------------------------------------- + + +def test_complete_objective_lands_in_audit_and_ome_xml( + tmp_path: Path, patched_reader +) -> None: + reader = _reader_with_metadata( + _lif_metadata_with_atl( + 'ObjectiveName="HC PL APO CS2 63x/1.40 OIL" ' + 'ObjectiveMag="63" NumericalAperture="1.4" Immersion="OIL" ' + 'WorkingDistance="140"' + ) + ) + patched_reader(reader) + out = tmp_path / "out" + + convert("/tmp/x.lif", out, pyramid_min_size=8) + + store = out / "scene0.ome.zarr" + + # (1) Audit — full dict with every field populated + audit = json.loads((store / "zarr.json").read_text())["attributes"]["zarrmony"] + objective = audit["per_scene"][0]["objective"] + assert objective == { + "nominal_magnification": 63, + "numerical_aperture": 1.4, + "immersion": "Oil", + "model": "HC PL APO CS2 63x/1.40 OIL", + "working_distance_um": 140, + } + + # (2) OME-XML — top-level with , and per-image + # + . + ome = from_xml((store / "OME" / "METADATA.ome.xml").read_text()) + assert len(ome.instruments) == 1 + instrument = ome.instruments[0] + assert len(instrument.objectives) == 1 + obj = instrument.objectives[0] + assert obj.nominal_magnification == 63.0 + assert obj.lens_na == 1.4 + assert obj.model == "HC PL APO CS2 63x/1.40 OIL" + assert obj.immersion.value == "Oil" + assert obj.working_distance == 140.0 + assert obj.working_distance_unit.value == "µm" + + image = ome.images[0] + assert image.instrument_ref is not None + assert image.instrument_ref.id == instrument.id + assert image.objective_settings is not None + assert image.objective_settings.id == obj.id + + +# --- partial objective block (magnification + NA only) --------------------- + + +def test_partial_objective_omits_missing_keys_from_audit( + tmp_path: Path, patched_reader +) -> None: + reader = _reader_with_metadata( + _lif_metadata_with_atl('ObjectiveMag="20" NumericalAperture="0.8"') + ) + patched_reader(reader) + out = tmp_path / "out" + + convert("/tmp/x.lif", out, pyramid_min_size=8) + + store = out / "scene0.ome.zarr" + audit = json.loads((store / "zarr.json").read_text())["attributes"]["zarrmony"] + + # Audit dict has ONLY the two extracted keys — no None-valued placeholders + # for model / immersion / working_distance_um. + objective = audit["per_scene"][0]["objective"] + assert objective == {"nominal_magnification": 20, "numerical_aperture": 0.8} + for missing in ("model", "immersion", "working_distance_um"): + assert missing not in objective + + # OME-XML: the partial element is still emitted with an ID and + # the two populated fields; the omitted OME attributes stay absent. + ome = from_xml((store / "OME" / "METADATA.ome.xml").read_text()) + obj = ome.instruments[0].objectives[0] + assert obj.nominal_magnification == 20.0 + assert obj.lens_na == 0.8 + assert obj.model is None + assert obj.immersion is None + assert obj.working_distance is None + + +# --- no objective info at all ---------------------------------------------- + + +def test_absent_objective_omits_key_from_audit_and_no_instrument( + tmp_path: Path, patched_reader +) -> None: + # LIF-shaped metadata with a scene but NO ATLConfocalSettingDefinition + # (nor element) — the extractor returns None, so the audit + # must OMIT the ``objective`` key entirely (not set it to null / {}), and + # the OME-XML must carry no at all. + reader = _reader_with_metadata(_lif_metadata_with_atl("")) + patched_reader(reader) + out = tmp_path / "out" + + convert("/tmp/x.lif", out, pyramid_min_size=8) + + store = out / "scene0.ome.zarr" + audit = json.loads((store / "zarr.json").read_text())["attributes"]["zarrmony"] + + assert "objective" not in audit["per_scene"][0] + + ome = from_xml((store / "OME" / "METADATA.ome.xml").read_text()) + assert ome.instruments == [] + assert ome.images[0].instrument_ref is None + assert ome.images[0].objective_settings is None diff --git a/tests/test_lif_per_tile.py b/tests/test_lif_per_tile.py index c5b91d9..2e27da9 100644 --- a/tests/test_lif_per_tile.py +++ b/tests/test_lif_per_tile.py @@ -221,7 +221,7 @@ def test_per_tile_audit_records_per_tile_and_tile_stores( # All three tile audits carry per_tile=true + the full tile_stores list. for tile_audit in result["stores"]: - assert tile_audit["audit_schema_version"] == 6 + assert tile_audit["audit_schema_version"] == 7 m = tile_audit["mosaic"] assert m["per_tile"] is True assert m["tile_count"] == 3 @@ -254,7 +254,7 @@ def test_per_tile_audit_round_trips_to_on_disk_attrs( with open(out / "Position_1" / "tile_X00Y00.ome.zarr" / "zarr.json") as f: root = json.load(f) audit = root["attributes"]["zarrmony"] - assert audit["audit_schema_version"] == AUDIT_SCHEMA_VERSION == 6 + assert audit["audit_schema_version"] == AUDIT_SCHEMA_VERSION == 7 assert audit["mosaic"]["per_tile"] is True assert audit["mosaic"]["tile_index"] == 0 assert len(audit["mosaic"]["tile_stores"]) == 3