From 4be004a13ef2fe81aaf429020a92163ea6c71e8e Mon Sep 17 00:00:00 2001 From: "Austin E. Y. T. Lefebvre" Date: Thu, 25 Jun 2026 15:34:05 -0700 Subject: [PATCH 1/5] test(fixtures): add pruned Leica LIF confocal scene XML (7ch) for channel extraction --- tests/fixtures/lif_confocal_7ch.xml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 tests/fixtures/lif_confocal_7ch.xml diff --git a/tests/fixtures/lif_confocal_7ch.xml b/tests/fixtures/lif_confocal_7ch.xml new file mode 100644 index 0000000..3e31c39 --- /dev/null +++ b/tests/fixtures/lif_confocal_7ch.xml @@ -0,0 +1,3 @@ + + +ChannelGroup0ChannelTypePhotonIntegrationBeamRoute40;0DetectorNameHyD S 1DyeNameLeica/DAPI (dsDNA bound)SequentialSettingIndex0DigitalGatingModeChannelGroup1ChannelTypePhotonIntegrationBeamRoute40;2DetectorNameHyD S 3DyeNameLeica/ALEXA 594SequentialSettingIndex0DigitalGatingModeChannelGroup2ChannelTypePhotonIntegrationBeamRoute40;4DetectorNameHyD R 5DyeNameLeica/ALEXA 750SequentialSettingIndex0DigitalGatingModeIntensityChannelGroup3ChannelTypePhotonIntegrationBeamRoute40;1DetectorNameHyD X 2DyeNameLeica/ALEXA 488SequentialSettingIndex1DigitalGatingModeIntensityChannelGroup4ChannelTypePhotonIntegrationBeamRoute40;3DetectorNameHyD X 4DyeNameLeica/ALEXA 647-R-PESequentialSettingIndex1DigitalGatingModeIntensityChannelGroup5ChannelTypePhotonIntegrationBeamRoute40;1DetectorNameHyD X 2DyeNameLeica/ALEXA 555SequentialSettingIndex2DigitalGatingModeIntensityChannelGroup6ChannelTypePhotonIntegrationBeamRoute40;3DetectorNameHyD X 4DyeNameLeica/ALEXA 700SequentialSettingIndex2DigitalGatingModeIntensity \ No newline at end of file From 9970aff07d0f7277f86e82960ee60239e66491c1 Mon Sep 17 00:00:00 2001 From: "Austin E. Y. T. Lefebvre" Date: Thu, 25 Jun 2026 15:34:05 -0700 Subject: [PATCH 2/5] feat(metadata): extract Leica LIF confocal channel identity (dye/excitation/emission/detector) Pure stdlib, hardened (DTD/entity-bomb fail-closed), bioio-free so it can lift into bioio-lif.ome_metadata. Resolves channels via LDM_Block_Sequential_List, dodging the phantom 620nm + duplicate settings blocks. Slice 1 of the channel-metadata run. --- src/zarrmony/metadata/lif_channels.py | 303 ++++++++++++++++++++++++++ tests/test_lif_channels.py | 190 ++++++++++++++++ 2 files changed, 493 insertions(+) create mode 100644 src/zarrmony/metadata/lif_channels.py create mode 100644 tests/test_lif_channels.py diff --git a/src/zarrmony/metadata/lif_channels.py b/src/zarrmony/metadata/lif_channels.py new file mode 100644 index 0000000..7a4ad45 --- /dev/null +++ b/src/zarrmony/metadata/lif_channels.py @@ -0,0 +1,303 @@ +"""Pure, stdlib-only per-channel identity extractor for Leica LIF scene XML. + +A single Leica ``.lif`` scene preserves its full acquisition settings as an XML +blob (the same text zarrmony copies verbatim to ``OME/source/raw.lif.xml``). +:func:`extract_channels` turns that blob into one identity dict per image +channel — dye, detector, excitation line, and emission band — without touching +``bioio``, ``ome_types``, or anything outside the standard library. Keeping it +dependency-free is deliberate: this is the load-bearing core later slices build +on, and it is meant to lift into ``bioio-lif``'s ``ome_metadata`` unchanged. + +The hard part is the JOIN, not the parsing. A Leica scene stores several copies +of its instrument settings, and only one set is the *real* acquisition: + +* ``LDM_Block_Sequential / LDM_Block_Sequential_List`` holds the genuine + sequential settings, in document order. A channel's ``SequentialSettingIndex`` + indexes directly into this list. This is the source of truth. +* ``LDM_Block_Sequential_Master`` and the top-level + ``Attachment[HardwareSetting]`` copies are reference/duplicate snapshots, NOT + real sequences. The Master copy in particular can carry a phantom laser line + that excites nothing in the acquired data; folding it in yields wrong answers. + We never read from them. + +Within a real sequence, excitation is recovered by spectral pairing: the active +laser lines (``LaserLineSetting`` with ``IntensityDev > 0``) are matched, in +ascending wavelength order, to the active detectors (``Detector`` with +``IsActive == "1"``) in ascending channel order. A channel's excitation is the +laser paired to its own physical detector channel. Emission comes from the +``MultiBand`` whose ``Channel`` matches that same physical channel. + +Hardening: the parser is fail-closed. Oversized input, DTDs / entity +definitions (the "billion laughs" expansion vector), external entities, and any +malformed or structurally-missing XML all yield ``[]`` — never an exception, +never an entity expansion, never a hang. +""" + +import math +import re +import xml.etree.ElementTree as ET + +# Refuse anything larger than this up front. A confocal scene blob is a few +# hundred KB; 32 MiB is generous headroom while still bounding parse work. +_MAX_BYTES = 32 * 1024 * 1024 + +# A LIF scene blob is plain element/attribute XML with no document type. Any +# DOCTYPE or entity declaration is therefore either malformed or hostile (the +# billion-laughs / external-entity vectors). stdlib ``ElementTree`` *does* +# expand internal entities, so we reject these textually before we ever parse. +_DOCTYPE_OR_ENTITY = re.compile(r" "ALEXA 594". +_DYE_VENDOR_PREFIX = "Leica/" + + +class _EntityRejectingTarget: + """ExpatBuilder target that refuses DTDs and entity definitions. + + Belt-and-suspenders alongside the textual pre-scan: even if a declaration + slipped past the regex, expat's ``entity_decl`` / ``unparsed_entity_decl`` + callbacks fire here and abort the parse instead of expanding anything. + """ + + def __init__(self) -> None: + self._builder = ET.TreeBuilder() + + # --- declarations we forbid ------------------------------------------- + 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") + + # --- normal tree construction ----------------------------------------- + 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. + + Returns ``None`` (never raises) on oversized input, DTD/entity content, + external entities, or any malformed XML. + """ + if not isinstance(scene_xml, str) or not scene_xml: + return None + # Size cap on the encoded bytes — char count alone understates memory. + 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: + # ParseError, ValueError from the target, recursion limits, etc. + return None + + +def _cp_map(channel_desc: ET.Element) -> dict[str, str]: + """Flatten a ``ChannelDescription``'s ``ChannelProperty`` Key/Value pairs.""" + props: dict[str, str] = {} + for prop in channel_desc.iter("ChannelProperty"): + key = prop.find("Key") + val = prop.find("Value") + if key is not None and key.text: + props[key.text.strip()] = (val.text or "").strip() if val is not None else "" + return props + + +def _to_number(text: str | None) -> int | float | None: + """Parse a numeric string, returning int when integral, else float, else None. + + Non-finite values (``inf`` / ``nan``) are rejected as ``None`` — they are + never valid wavelengths and would otherwise break ``round``. + """ + 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 _sequence_blocks(root: ET.Element) -> list[ET.Element]: + """The real acquisition sequences, in document order. + + These are the ``ATLConfocalSettingDefinition`` children of + ``LDM_Block_Sequential / LDM_Block_Sequential_List`` — and ONLY those. The + ``LDM_Block_Sequential_Master`` block and the top-level + ``Attachment[HardwareSetting]`` copies are reference/duplicate snapshots and + are deliberately excluded; ``SequentialSettingIndex`` indexes into this list. + """ + blocks: list[ET.Element] = [] + # ``iter`` walks in document order, so each list's settings stay ordered and + # multiple sequential lists (rare) concatenate in the order they appear. + for seq_list in root.iter("LDM_Block_Sequential_List"): + for atl in seq_list.iter("ATLConfocalSettingDefinition"): + blocks.append(atl) + return blocks + + +def _detector_channel_map(root: ET.Element) -> dict[str, int]: + """Map every ``Detector`` ``Name`` to its physical ``Channel`` number. + + The mapping (e.g. ``HyD S 1`` -> 1, ``HyD X 2`` -> 2) is consistent across + every block, so we scan the whole document to be robust to partial blocks. + """ + mapping: dict[str, int] = {} + for det in root.iter("Detector"): + name = det.attrib.get("Name") + channel = det.attrib.get("Channel") + if not name or channel is None: + continue + try: + mapping[name] = int(channel) + except ValueError: + continue + return mapping + + +def _active_lasers(block: ET.Element) -> list[int | float]: + """Excited laser lines (``IntensityDev > 0``) for a sequence, ascending.""" + lasers: list[int | float] = [] + for laser in block.iter("LaserLineSetting"): + intensity = _to_number(laser.attrib.get("IntensityDev")) + line = _to_number(laser.attrib.get("LaserLine")) + if intensity is not None and intensity > 0 and line is not None: + lasers.append(line) + return sorted(lasers) + + +def _active_detector_channels(block: ET.Element) -> list[int]: + """Physical channels of the active detectors (``IsActive == "1"``), ascending.""" + channels: list[int] = [] + for det in block.iter("Detector"): + if det.attrib.get("IsActive") != "1": + continue + channel = det.attrib.get("Channel") + if channel is None: + continue + try: + channels.append(int(channel)) + except ValueError: + continue + return sorted(channels) + + +def _excitation_for(block: ET.Element, physical_channel: int | None) -> int | float | None: + """Excitation line for ``physical_channel`` within one sequence. + + Spectral pairing: the i-th lowest active laser drives the i-th lowest active + detector. Find this channel's position among the active detectors, then read + the laser at the same position. + """ + if physical_channel is None: + return None + lasers = _active_lasers(block) + detectors = _active_detector_channels(block) + if physical_channel not in detectors: + return None + position = detectors.index(physical_channel) + if position >= len(lasers): + return None + return lasers[position] + + +def _emission_for( + block: ET.Element, physical_channel: int | None +) -> tuple[int | None, int | None]: + """``(low, high)`` emission band (rounded nm) for ``physical_channel``.""" + if physical_channel is None: + return None, None + for band in block.iter("MultiBand"): + channel = _to_number(band.attrib.get("Channel")) + if channel is None or int(channel) != physical_channel: + continue + left = _to_number(band.attrib.get("LeftWorld")) + right = _to_number(band.attrib.get("RightWorld")) + low = round(left) if left is not None else None + high = round(right) if right is not None else None + return low, high + return None, None + + +def _strip_dye_prefix(dye: str | None) -> str | None: + """Drop the leading ``"Leica/"`` vendor prefix from a dye name.""" + if dye is None: + return None + return dye.removeprefix(_DYE_VENDOR_PREFIX) + + +def extract_channels(scene_xml: str) -> list[dict]: + """Extract per-channel identity from one Leica LIF scene XML string. + + Returns one dict per image channel, in acquisition (document) order, each + with exactly the keys ``index``, ``dye``, ``fluor``, ``detector``, + ``excitation_nm``, ``emission_low_nm``, ``emission_high_nm``. + + Fail-closed: any parse failure, unsafe input, or missing structure yields + ``[]``. Individual undeterminable fields degrade to ``None`` rather than + dropping the channel. + """ + root = _safe_parse(scene_xml) + if root is None: + return [] + + try: + sequences = _sequence_blocks(root) + detector_to_channel = _detector_channel_map(root) + + channels: list[dict] = [] + for index, channel_desc in enumerate(root.iter("ChannelDescription")): + props = _cp_map(channel_desc) + + dye = _strip_dye_prefix(props.get("DyeName") or None) + detector_name = props.get("DetectorName") or None + physical_channel = ( + detector_to_channel.get(detector_name) if detector_name else None + ) + + # Resolve which real sequence acquired this channel. + block = None + seq_index = _to_number(props.get("SequentialSettingIndex")) + if seq_index is not None and 0 <= int(seq_index) < len(sequences): + block = sequences[int(seq_index)] + + if block is not None: + excitation = _excitation_for(block, physical_channel) + emission_low, emission_high = _emission_for(block, physical_channel) + else: + excitation = None + emission_low = emission_high = None + + channels.append( + { + "index": index, + "dye": dye, + "fluor": dye, + "detector": detector_name, + "excitation_nm": excitation, + "emission_low_nm": emission_low, + "emission_high_nm": emission_high, + } + ) + return channels + except Exception: + # Any unexpected structural surprise stays fail-closed. + return [] diff --git a/tests/test_lif_channels.py b/tests/test_lif_channels.py new file mode 100644 index 0000000..ebee86f --- /dev/null +++ b/tests/test_lif_channels.py @@ -0,0 +1,190 @@ +"""Tests for the stdlib-only Leica LIF channel extractor. + +Exercises the confocal fixture (the JOIN + phantom-block disambiguation), the +fail-closed hardening (entity bombs, XXE, malformed/oversized input), and +graceful degradation on partial metadata. +""" + +from pathlib import Path + +from zarrmony.metadata.lif_channels import extract_channels + +FIXTURE = Path(__file__).parent / "fixtures" / "lif_confocal_7ch.xml" + + +def _channels() -> list[dict]: + return extract_channels(FIXTURE.read_text(encoding="utf-8")) + + +# --- the confocal fixture: identity + the JOIN ----------------------------- + + +def test_extracts_exactly_seven_channels_in_order() -> None: + channels = _channels() + assert len(channels) == 7 + assert [c["index"] for c in channels] == [0, 1, 2, 3, 4, 5, 6] + + +def test_each_channel_has_exactly_the_contract_keys() -> None: + expected = { + "index", + "dye", + "fluor", + "detector", + "excitation_nm", + "emission_low_nm", + "emission_high_nm", + } + for channel in _channels(): + assert set(channel) == expected + + +def test_dyes_have_vendor_prefix_stripped() -> None: + assert [c["dye"] for c in _channels()] == [ + "DAPI (dsDNA bound)", + "ALEXA 594", + "ALEXA 750", + "ALEXA 488", + "ALEXA 647-R-PE", + "ALEXA 555", + "ALEXA 700", + ] + + +def test_fluor_mirrors_dye() -> None: + for channel in _channels(): + assert channel["fluor"] == channel["dye"] + + +def test_detectors() -> None: + assert [c["detector"] for c in _channels()] == [ + "HyD S 1", + "HyD S 3", + "HyD R 5", + "HyD X 2", + "HyD X 4", + "HyD X 2", + "HyD X 4", + ] + + +def test_excitation_uses_real_sequences_not_phantom() -> None: + # The trap: the Master block's phantom 620 nm line and the duplicate + # top-level Attachment copy must not corrupt the spectral pairing. + excitations = [c["excitation_nm"] for c in _channels()] + assert excitations == [405, 590, 753, 499, 653, 553, 696] + assert 620 not in excitations + assert all(e is not None for e in excitations) + + +def test_excitation_values_are_ints_when_integral() -> None: + for channel in _channels(): + assert isinstance(channel["excitation_nm"], int) + + +def test_emission_bands_rounded() -> None: + bands = [(c["emission_low_nm"], c["emission_high_nm"]) for c in _channels()] + assert bands == [ + (430, 499), + (601, 640), + (768, 829), + (506, 548), + (663, 688), + (562, 600), + (706, 749), + ] + + +# --- fail-closed hardening -------------------------------------------------- + + +def test_billion_laughs_returns_empty_fast() -> None: + bomb = '\n\n' + prev = "a" + for i in range(2, 12): + bomb += f' \n' # nested expansion + prev = f"a{i}" + bomb += "]>\n&a11;" + assert extract_channels(bomb) == [] + + +def test_external_entity_returns_empty() -> None: + xxe = ( + '' + ']>' + "&x;" + ) + assert extract_channels(xxe) == [] + + +def test_doctype_is_rejected_even_without_entities() -> None: + assert extract_channels('') == [] + + +def test_malformed_xml_returns_empty() -> None: + assert extract_channels(" None: + assert extract_channels("") == [] + assert extract_channels(None) == [] # type: ignore[arg-type] + + +def test_oversized_input_returns_empty() -> None: + huge = "" + ("x" * (33 * 1024 * 1024)) + "" + assert extract_channels(huge) == [] + + +def test_valid_xml_without_channels_returns_empty() -> None: + assert extract_channels("") == [] + + +# --- graceful degradation on partial metadata ------------------------------ + + +def test_non_finite_band_values_degrade_without_dropping_channel() -> None: + # A pathological inf/nan emission must not crash round() or drop the + # channel — the band degrades to None while excitation is still kept. + xml = """ + + DetectorNameHyD S 1 + SequentialSettingIndex0 + + + + + + + + + + """ + out = extract_channels(xml) + assert len(out) == 1 + assert out[0]["excitation_nm"] == 488 + assert out[0]["emission_low_nm"] is None + assert out[0]["emission_high_nm"] is None + + +def test_missing_fields_degrade_to_none_without_dropping_channel() -> None: + xml = """ + + DetectorNameHyD X 2 + SequentialSettingIndex0 + + + + + + + + """ + out = extract_channels(xml) + assert len(out) == 1 + assert out[0]["detector"] == "HyD X 2" + assert out[0]["dye"] is None + assert out[0]["fluor"] is None + assert out[0]["excitation_nm"] is None + assert out[0]["emission_low_nm"] is None + assert out[0]["emission_high_nm"] is None From 33388deb6e036f3bbbc8df8e56d8d421f9a1fb27 Mon Sep 17 00:00:00 2001 From: "Austin E. Y. T. Lefebvre" Date: Thu, 25 Jun 2026 15:34:05 -0700 Subject: [PATCH 3/5] feat(metadata): write LIF channel identity into OME-XML + omero labels For Leica LIF, the per-scene path now builds a real OME Image (dye name, excitation & emission wavelengths) and omero labels like 'ALEXA 594 (590 nm)' instead of a MetadataOnly stub. LIF-only (keyed on scene_root), fail-safe (any error falls back to the prior name-based path), channel-count vs SizeC guarded, CZI/ND2 untouched. Slice 2 of the channel-metadata run. --- src/zarrmony/api.py | 114 ++++++++++- src/zarrmony/metadata/lif_channels.py | 128 ++++++++++++ tests/test_api_lif_wiring.py | 66 ++++++ tests/test_lif_projections.py | 284 ++++++++++++++++++++++++++ 4 files changed, 583 insertions(+), 9 deletions(-) create mode 100644 tests/test_api_lif_wiring.py create mode 100644 tests/test_lif_projections.py diff --git a/src/zarrmony/api.py b/src/zarrmony/api.py index aee63ce..507c93e 100644 --- a/src/zarrmony/api.py +++ b/src/zarrmony/api.py @@ -26,6 +26,11 @@ ZarrmonyError, ) from zarrmony.metadata.channel_colors import colors_for_channels +from zarrmony.metadata.lif_channels import ( + channels_to_ome_channels, + channels_to_omero, + extract_channels, +) from zarrmony.metadata.model import UserMetadata from zarrmony.naming import resolve_scene_dirnames from zarrmony.readers.default import derive_bioio_distribution @@ -188,6 +193,81 @@ def _channels_for_scene( ] +def _scene_channel_count(reader: Any) -> int: + """The current scene's C size, from the reader's public xarray surface. + + Mirrors how :func:`inspect` reads dims/sizes (no coupling to writer + internals). A scene with no ``C`` dim has one implicit channel. + """ + xarr = reader.xarray_dask_data + 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]: + """The LIF-vs-other decision, in one place. + + Returns ``(extracted, omero_channels)`` when ``reader`` is a LIF reader + (exposes ``scene_root``), its 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. + + 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 (corruption, partial metadata), we decline the projection rather + than label a 2-channel image with 7 names. + + Fail-safe: any failure — not a LIF reader, no/garbage scene XML, an empty or + count-mismatched extraction, or an unexpected error — returns + ``(None, None)`` so callers fall back cleanly to the existing name-based + path. Metadata never crashes a conversion. + """ + try: + # NB: ``scene_root`` is a *property* on bioio_lif.Reader that RAISES + # (ValueError) for ordinary non-plate scenes — the common confocal case — + # so it must be read inside the try, not via getattr() (which only + # swallows AttributeError). A raise here is a clean fall-back, not a crash. + scene_root = getattr(reader, "scene_root", None) + if scene_root is None: + return None, None + scene_xml = ET.tostring(scene_root, encoding="unicode") + extracted = extract_channels(scene_xml) + if not extracted or len(extracted) != _scene_channel_count(reader): + return None, None + omero_channels = [ + Channel(label=o["label"], color=o["color"]) + for o in channels_to_omero(extracted) + ] + return extracted, omero_channels + except Exception: # noqa: BLE001 — never break a conversion over metadata + return None, None + + +def _lif_ome_image( + extracted: list[dict], scene_index: int, name: str, scene_record: dict +) -> Image | None: + """A real per-scene OME ``Image`` carrying the extracted channel identities. + + Sizes come from ``scene_record`` (exactly as :func:`_stub_image` does); the + canonical ```` elements come from :func:`channels_to_ome_channels`. + ``extracted`` was already count-checked against the scene's C size in + :func:`_lif_scene_channels`; the ``SizeC`` assertion here is belt-and- + suspenders. Returns ``None`` on any surprise so the caller falls back to the + stub Image (with name-based omero, the existing behavior). + """ + try: + image = _stub_image(scene_index, name, scene_record) + ome_channels = channels_to_ome_channels(extracted) + if len(ome_channels) != image.pixels.size_c: + return None + image.pixels.channels = ome_channels + return image + 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" @@ -391,7 +471,16 @@ def _convert_per_scene( ) continue - channels = _channels_for_scene(reader, channel_colors) + # 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) + channels = ( + lif_omero_channels + if lif_omero_channels is not None + else _channels_for_scene(reader, channel_colors) + ) scene_record = write_scene( reader, @@ -410,16 +499,23 @@ def _convert_per_scene( scene_user_md = per_scene_user_metadata[scene_name] scene_record["user_metadata"] = scene_user_md - ome_image, warning = _try_get_ome_image(reader, scene_index) metadata_warnings: list[dict] = [] - if warning is not None: - metadata_warnings.append(warning) - warnings.warn( - f"scene {scene_index} ({scene_name}): {warning['error']}", - ExtractorWarning, - stacklevel=2, + ome_image: Image | None = None + if lif_extracted is not None: + ome_image = _lif_ome_image( + lif_extracted, scene_index, scene_name, scene_record ) - ome_image = _stub_image(scene_index, scene_name, scene_record) + if ome_image is None: + # Non-LIF, or the LIF Image couldn't be built — existing behavior. + ome_image, warning = _try_get_ome_image(reader, scene_index) + if warning is not None: + metadata_warnings.append(warning) + warnings.warn( + f"scene {scene_index} ({scene_name}): {warning['error']}", + ExtractorWarning, + stacklevel=2, + ) + ome_image = _stub_image(scene_index, scene_name, scene_record) ome_xml = build_ome_xml_for_scene(ome_image) write_per_scene_metadata( diff --git a/src/zarrmony/metadata/lif_channels.py b/src/zarrmony/metadata/lif_channels.py index 7a4ad45..2fe922f 100644 --- a/src/zarrmony/metadata/lif_channels.py +++ b/src/zarrmony/metadata/lif_channels.py @@ -31,11 +31,26 @@ definitions (the "billion laughs" expansion vector), external entities, and any malformed or structurally-missing XML all yield ``[]`` — never an exception, never an entity expansion, never a hang. + +Projections (below :func:`extract_channels`) — :func:`channels_to_omero` and +:func:`channels_to_ome_channels` — map those identity dicts into the two shapes +consumers actually read: the OME-XML ```` element (canonical) and the +omero display ``label``/``color`` (derived). They are deliberately *zarrmony-side* +and so are allowed the ``ome_types`` + ``zarrmony.metadata.channel_colors`` +dependencies the core forbids itself; those imports are kept local to the +projections so the bioio-portable core can still be imported and called with the +standard library alone. """ +from __future__ import annotations + import math import re import xml.etree.ElementTree as ET +from typing import TYPE_CHECKING + +if TYPE_CHECKING: # pragma: no cover - typing only + from ome_types.model import Channel # Refuse anything larger than this up front. A confocal scene blob is a few # hundred KB; 32 MiB is generous headroom while still bounding parse work. @@ -301,3 +316,116 @@ def extract_channels(scene_xml: str) -> list[dict]: except Exception: # Any unexpected structural surprise stays fail-closed. return [] + + +# --------------------------------------------------------------------------- +# Projections: identity dicts -> the shapes consumers read. +# +# These are zarrmony-side, NOT the bioio-portable core, so they are allowed +# the ``ome_types`` + ``channel_colors`` dependencies ``extract_channels`` +# forbids itself. Those imports stay local to keep the core stdlib-only on +# import. Each is a small pure function over the dicts ``extract_channels`` +# returns; they degrade per-field (omit/None) rather than inventing values, so +# garbage or partial input never raises. +# --------------------------------------------------------------------------- + +# A trailing " (...)" parenthetical qualifier on a dye name, e.g. the +# "(dsDNA bound)" in "DAPI (dsDNA bound)". Anchored to the end so an interior +# parenthesis in an (unusual) dye name is left intact. +_TRAILING_PARENTHETICAL = re.compile(r"\s*\([^()]*\)\s*$") + + +def _clean_dye(dye: str | None) -> str | None: + """Strip a trailing ``" (...)"`` qualifier from a dye name. + + ``"DAPI (dsDNA bound)"`` -> ``"DAPI"``; ``"ALEXA 594"`` -> ``"ALEXA 594"``; + ``None`` -> ``None``. Leaves a name with no trailing parenthetical untouched. + """ + if dye is None: + return None + return _TRAILING_PARENTHETICAL.sub("", dye).strip() or None + + +def _color_for(cleaned_dye: str | None, index: int) -> str: + """Heuristic hex color for a (cleaned) dye, palette-fallback by index. + + Imported locally so the bioio-portable core stays stdlib-only on import: + ``zarrmony.metadata.channel_colors`` won't exist once ``extract_channels`` + is lifted into ``bioio-lif``, but these projections stay zarrmony-side. + """ + from zarrmony.metadata.channel_colors import color_for_channel + + return color_for_channel(cleaned_dye or "", index) + + +def _omero_label( + cleaned_dye: str | None, excitation_nm: int | float | None +) -> str | None: + """The omero label per the ladder: dye + excitation, then either, then None.""" + if cleaned_dye and excitation_nm is not None: + return f"{cleaned_dye} ({excitation_nm} nm)" + if cleaned_dye: + return cleaned_dye + if excitation_nm is not None: + return f"{excitation_nm} nm" + return None + + +def channels_to_omero(channels: list[dict]) -> list[dict]: + """Project identity dicts into omero display ``{"label", "color"}`` dicts. + + One dict per input channel, in order. ``label`` follows the dye/excitation + ladder (``None`` only when neither is known); ``color`` is the cleaned dye's + heuristic hex (falling back to the palette by index), so it is always a valid + six-hex string even for an unnamed channel. + """ + out: list[dict] = [] + for index, ch in enumerate(channels): + cleaned_dye = _clean_dye(ch.get("dye")) + out.append( + { + "label": _omero_label(cleaned_dye, ch.get("excitation_nm")), + "color": _color_for(cleaned_dye, index), + } + ) + return out + + +def channels_to_ome_channels(channels: list[dict]) -> list[Channel]: + """Project identity dicts into canonical ome_types ``Channel`` elements. + + One ``Channel`` per input channel: ``id=Channel:0:``, ``name`` the + cleaned dye, ``fluor`` the full (un-cleaned) dye, excitation/emission with + ``nm`` units, and the same hex ``color`` as :func:`channels_to_omero`. Any + field whose source is ``None`` is omitted entirely rather than set to a + bogus value, so the element never claims wavelengths it doesn't have. + """ + # Local import: the bioio-portable core stays stdlib-only on module import. + from ome_types.model import Channel + + out: list[Channel] = [] + for index, ch in enumerate(channels): + cleaned_dye = _clean_dye(ch.get("dye")) + fields: dict = { + "id": f"Channel:0:{index}", + "color": _color_for(cleaned_dye, index), + } + if cleaned_dye is not None: + fields["name"] = cleaned_dye + fluor = ch.get("fluor") + if fluor is not None: + fields["fluor"] = fluor + + excitation = ch.get("excitation_nm") + if excitation is not None: + fields["excitation_wavelength"] = excitation + fields["excitation_wavelength_unit"] = "nm" + + low = ch.get("emission_low_nm") + high = ch.get("emission_high_nm") + if low is not None and high is not None: + fields["emission_wavelength"] = round((low + high) / 2) + fields["emission_wavelength_unit"] = "nm" + + out.append(Channel(**fields)) + return out diff --git a/tests/test_api_lif_wiring.py b/tests/test_api_lif_wiring.py new file mode 100644 index 0000000..d027d47 --- /dev/null +++ b/tests/test_api_lif_wiring.py @@ -0,0 +1,66 @@ +"""LIF api-wiring fail-safe regression. + +The LIF channel-metadata branch must never crash a conversion and must decline +cleanly on: a non-LIF reader, a ``scene_root`` property that *raises* (the common +non-plate confocal case in bioio_lif), or a channel-count vs SizeC mismatch. +""" +import xml.etree.ElementTree as ET +from pathlib import Path + +from zarrmony.api import _lif_scene_channels + +FIX = Path(__file__).parent / "fixtures" / "lif_confocal_7ch.xml" + + +class _Xarr: + def __init__(self, c): + self.dims = ("C",) if c is not None else () + self.sizes = {"C": c} if c is not None else {} + + +class _Raising: + """bioio_lif's scene_root raises ValueError for ordinary (non-plate) scenes.""" + + xarray_dask_data = _Xarr(7) + + @property + def scene_root(self): + raise ValueError("scene is not a plate well") + + +class _NonLif: + xarray_dask_data = _Xarr(7) + + +class _Good: + xarray_dask_data = _Xarr(7) + + @property + def scene_root(self): + return ET.parse(FIX).getroot() + + +class _CountMismatch: + xarray_dask_data = _Xarr(2) + + @property + def scene_root(self): + return ET.parse(FIX).getroot() + + +def test_raising_scene_root_falls_back(): + assert _lif_scene_channels(_Raising()) == (None, None) + + +def test_non_lif_falls_back(): + assert _lif_scene_channels(_NonLif()) == (None, None) + + +def test_count_mismatch_declines(): + assert _lif_scene_channels(_CountMismatch()) == (None, None) + + +def test_happy_path_wiring(): + extracted, omero = _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)" diff --git a/tests/test_lif_projections.py b/tests/test_lif_projections.py new file mode 100644 index 0000000..ea44819 --- /dev/null +++ b/tests/test_lif_projections.py @@ -0,0 +1,284 @@ +"""Tests for the zarrmony-side LIF channel projections + api wiring. + +Covers: +- ``channels_to_omero`` — the label ladder, the trailing-parenthetical strip on + the dye, and the always-valid hex color. +- ``channels_to_ome_channels`` — cleaned name vs full fluor, excitation/emission + units, band-center rounding, and field-omission when a source is None. +- the fail-safe contract: empty / garbage input degrades, never raises. +- the api wiring: a LIF-shaped reader (exposes ``scene_root``) gets real + ```` elements + omero label/color in its store; a non-LIF reader is + untouched and a LIF whose channel count disagrees with the data falls back. +""" + +import xml.etree.ElementTree as ET +from pathlib import Path + +import zarr +from ome_types import from_xml + +from tests.conftest import FakeReader +from zarrmony import api as api_module +from zarrmony import convert +from zarrmony.metadata.lif_channels import ( + channels_to_ome_channels, + channels_to_omero, + extract_channels, +) +from zarrmony.readers.plugin import ReaderPlugin + +FIXTURE = Path(__file__).parent / "fixtures" / "lif_confocal_7ch.xml" + + +def _channels() -> list[dict]: + return extract_channels(FIXTURE.read_text(encoding="utf-8")) + + +# --- channels_to_omero ------------------------------------------------------ + + +def test_omero_labels_match_the_confocal_fixture() -> None: + labels = [o["label"] for o in channels_to_omero(_channels())] + assert labels == [ + "DAPI (405 nm)", + "ALEXA 594 (590 nm)", + "ALEXA 750 (753 nm)", + "ALEXA 488 (499 nm)", + "ALEXA 647-R-PE (653 nm)", + "ALEXA 555 (553 nm)", + "ALEXA 700 (696 nm)", + ] + + +def test_omero_colors_are_always_valid_hex6() -> None: + for o in channels_to_omero(_channels()): + assert isinstance(o["color"], str) + assert len(o["color"]) == 6 + int(o["color"], 16) # parses as hex + + +def test_omero_label_ladder() -> None: + rows = [ + # dye + excitation -> "dye (exc nm)" with the parenthetical stripped + ({"dye": "DAPI (dsDNA bound)", "excitation_nm": 405}, "DAPI (405 nm)"), + # dye only -> cleaned dye + ({"dye": "ALEXA 594", "excitation_nm": None}, "ALEXA 594"), + # excitation only -> "exc nm" + ({"dye": None, "excitation_nm": 488}, "488 nm"), + # neither -> None + ({"dye": None, "excitation_nm": None}, None), + # a dye that is *only* a parenthetical collapses to None, not "" -> + # excitation (if any) takes over per the ladder + ({"dye": "(unmixed)", "excitation_nm": 561}, "561 nm"), + ] + for ch, expected in rows: + assert channels_to_omero([ch])[0]["label"] == expected + + +def test_omero_color_uses_cleaned_dye_not_full_name() -> None: + # "DAPI (dsDNA bound)" must map to DAPI's blue via the cleaned name, the + # same as a bare "DAPI" would — not the palette fallback. + cleaned = channels_to_omero([{"dye": "DAPI (dsDNA bound)"}])[0]["color"] + bare = channels_to_omero([{"dye": "DAPI"}])[0]["color"] + assert cleaned == bare + + +# --- channels_to_ome_channels ----------------------------------------------- + + +def test_ome_channels_shape_and_identity() -> None: + ome = channels_to_ome_channels(_channels()) + assert len(ome) == 7 + assert [c.id for c in ome] == [f"Channel:0:{i}" for i in range(7)] + # name is the *cleaned* dye; fluor is the *full* dye. + assert ome[0].name == "DAPI" + assert ome[0].fluor == "DAPI (dsDNA bound)" + # excitation carried with nm units. + assert float(ome[0].excitation_wavelength) == 405.0 + assert str(ome[0].excitation_wavelength_unit) in ("nm", "UnitsLength.NANOMETER") + # emission at band center (430..499 -> 464.5 -> 464), with nm units. + assert float(ome[0].emission_wavelength) == 464.0 + assert str(ome[0].emission_wavelength_unit) in ("nm", "UnitsLength.NANOMETER") + + +def test_ome_channels_omit_fields_when_source_is_none() -> None: + # detector-only channel: no dye/fluor, no excitation, no emission band. + ch = { + "index": 0, + "dye": None, + "fluor": None, + "detector": "HyD X 2", + "excitation_nm": None, + "emission_low_nm": None, + "emission_high_nm": None, + } + c = channels_to_ome_channels([ch])[0] + assert c.id == "Channel:0:0" + assert c.name is None + assert c.fluor is None + assert c.excitation_wavelength is None + assert c.emission_wavelength is None + # color is still set (palette fallback), never None. + assert c.color is not None + + +def test_ome_channels_omit_emission_when_band_half_missing() -> None: + ch = {"emission_low_nm": 500, "emission_high_nm": None} + assert channels_to_ome_channels([ch])[0].emission_wavelength is None + + +# --- fail-safe: empty / garbage never raises -------------------------------- + + +def test_empty_input_yields_empty_projections() -> None: + assert channels_to_omero([]) == [] + assert channels_to_ome_channels([]) == [] + + +def test_garbage_dicts_degrade_without_raising() -> None: + garbage = [{}, {"dye": ""}, {"excitation_nm": None, "dye": None}] + omero = channels_to_omero(garbage) + assert [o["label"] for o in omero] == [None, None, None] + assert all(isinstance(o["color"], str) and len(o["color"]) == 6 for o in omero) + # ome_types projection also survives empty dicts. + assert len(channels_to_ome_channels(garbage)) == 3 + + +# --- api wiring ------------------------------------------------------------- + + +class FakeLifReader(FakeReader): + """A LIF-shaped FakeReader: adds the ``scene_root`` the api keys off.""" + + def __init__(self, *, scene_xml: str, **kwargs) -> None: + super().__init__(**kwargs) + self._scene_xml = scene_xml + + @property + def scene_root(self) -> ET.Element: + return ET.fromstring(self._scene_xml) + + +def _install(monkeypatch, reader: FakeReader) -> None: + plugin = ReaderPlugin( + name="bioio-lif", + match=lambda _p: 100, + open=lambda _p: object(), + distribution="bioio-lif", + source="builtin", + ) + monkeypatch.setattr(api_module, "get_reader", lambda _path: (reader, plugin, 100)) + + +def _md() -> dict: + return {"microscope": "Stellaris", "modality": "fluorescence"} + + +def test_api_lif_path_writes_real_channels(tmp_path: Path, monkeypatch) -> None: + xml = FIXTURE.read_text(encoding="utf-8") + reader = FakeLifReader( + scene_xml=xml, + scenes=["scene0"], + dims="CYX", + shape=(7, 16, 16), # SizeC must equal the 7 extracted channels + ) + _install(monkeypatch, reader) + out = tmp_path / "out" + + convert("/tmp/x.lif", out, metadata=_md(), pyramid_min_size=8) + + store = out / "scene0.ome.zarr" + + # OME-XML carries real elements with the cleaned names + waves. + parsed = from_xml((store / "OME" / "METADATA.ome.xml").read_text()) + chans = parsed.images[0].pixels.channels + assert [c.name for c in chans] == [ + "DAPI", + "ALEXA 594", + "ALEXA 750", + "ALEXA 488", + "ALEXA 647-R-PE", + "ALEXA 555", + "ALEXA 700", + ] + assert float(chans[0].excitation_wavelength) == 405.0 + assert float(chans[0].emission_wavelength) == 464.0 + assert chans[0].fluor == "DAPI (dsDNA bound)" + + # omero (display) label/color landed in the zarr group attrs. + g = zarr.open_group(str(store), mode="r") + omero = g.attrs["ome"]["omero"] + labels = [c["label"] for c in omero["channels"]] + assert labels[0] == "DAPI (405 nm)" + assert labels[1] == "ALEXA 594 (590 nm)" + for c in omero["channels"]: + assert isinstance(c["color"], str) and len(c["color"]) == 6 + + +def test_api_non_lif_reader_is_untouched(tmp_path: Path, monkeypatch) -> None: + # No scene_root -> the LIF decision is a no-op and the name-based path runs. + reader = FakeReader( + scenes=["s"], + dims="CYX", + shape=(2, 16, 16), + channel_names=["DAPI", "GFP"], + ) + _install(monkeypatch, reader) + out = tmp_path / "out" + + convert("/tmp/x.czi", out, metadata=_md(), pyramid_min_size=8) + + store = out / "s.ome.zarr" + g = zarr.open_group(str(store), mode="r") + labels = [c["label"] for c in g.attrs["ome"]["omero"]["channels"]] + # name-based omero, exactly as before this slice. + assert labels == ["DAPI", "GFP"] + # OME-XML uses the reader's ome_metadata stub (no LIF identity). + parsed = from_xml((store / "OME" / "METADATA.ome.xml").read_text()) + assert parsed.images[0].name == "s" + + +def test_api_lif_channel_count_mismatch_falls_back(tmp_path: Path, monkeypatch) -> None: + # 7 channels extracted but the data only has SizeC=2: don't write an + # OME-XML that contradicts the array. Fall back to the existing path. + xml = FIXTURE.read_text(encoding="utf-8") + reader = FakeLifReader( + scene_xml=xml, + scenes=["scene0"], + dims="CYX", + shape=(2, 16, 16), + ) + _install(monkeypatch, reader) + out = tmp_path / "out" + + # Must not raise; conversion completes. + convert("/tmp/x.lif", out, metadata=_md(), pyramid_min_size=8) + + store = out / "scene0.ome.zarr" + parsed = from_xml((store / "OME" / "METADATA.ome.xml").read_text()) + # SizeC honors the data (2), and the contradictory 7-channel identity was + # NOT forced in (channel count, if any, never exceeds SizeC). + assert parsed.images[0].pixels.size_c == 2 + assert len(parsed.images[0].pixels.channels) <= 2 + # The omero block also stays consistent: never 7 labels on a 2-channel image. + g = zarr.open_group(str(store), mode="r") + assert len(g.attrs["ome"]["omero"]["channels"]) == 2 + + +def test_api_lif_with_garbage_scene_root_falls_back( + tmp_path: Path, monkeypatch +) -> None: + # scene_root present but yields no channels -> graceful fallback, no crash. + reader = FakeLifReader( + scene_xml="", + scenes=["s"], + dims="CYX", + shape=(1, 16, 16), + ) + _install(monkeypatch, reader) + out = tmp_path / "out" + + convert("/tmp/x.lif", out, metadata=_md(), pyramid_min_size=8) + + store = out / "s.ome.zarr" + assert (store / "OME" / "METADATA.ome.xml").exists() From afbbcf2fd5f96ecf70863063abf5629a490ed7fb Mon Sep 17 00:00:00 2001 From: "Austin E. Y. T. Lefebvre" Date: Thu, 25 Jun 2026 15:34:05 -0700 Subject: [PATCH 4/5] style: black-format new metadata code; prettier-ignore upstream drafts --- src/zarrmony/metadata/lif_channels.py | 8 ++++++-- tests/test_api_lif_wiring.py | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/zarrmony/metadata/lif_channels.py b/src/zarrmony/metadata/lif_channels.py index 2fe922f..2f75ce8 100644 --- a/src/zarrmony/metadata/lif_channels.py +++ b/src/zarrmony/metadata/lif_channels.py @@ -130,7 +130,9 @@ def _cp_map(channel_desc: ET.Element) -> dict[str, str]: key = prop.find("Key") val = prop.find("Value") if key is not None and key.text: - props[key.text.strip()] = (val.text or "").strip() if val is not None else "" + props[key.text.strip()] = ( + (val.text or "").strip() if val is not None else "" + ) return props @@ -215,7 +217,9 @@ def _active_detector_channels(block: ET.Element) -> list[int]: return sorted(channels) -def _excitation_for(block: ET.Element, physical_channel: int | None) -> int | float | None: +def _excitation_for( + block: ET.Element, physical_channel: int | None +) -> int | float | None: """Excitation line for ``physical_channel`` within one sequence. Spectral pairing: the i-th lowest active laser drives the i-th lowest active diff --git a/tests/test_api_lif_wiring.py b/tests/test_api_lif_wiring.py index d027d47..dc88118 100644 --- a/tests/test_api_lif_wiring.py +++ b/tests/test_api_lif_wiring.py @@ -4,6 +4,7 @@ cleanly on: a non-LIF reader, a ``scene_root`` property that *raises* (the common non-plate confocal case in bioio_lif), or a channel-count vs SizeC mismatch. """ + import xml.etree.ElementTree as ET from pathlib import Path From 8bd694a3f1e989e58fbdc629f0186bfdcd683c50 Mon Sep 17 00:00:00 2001 From: "Austin E. Y. T. Lefebvre" Date: Fri, 26 Jun 2026 15:21:37 -0700 Subject: [PATCH 5/5] fix(metadata): make confocal LIF channel identity engage for non-plate scenes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _lif_scene_channels located each scene's settings XML via reader.scene_root, bioio-lif's plate row/column locator, which raises ValueError for ordinary non-plate confocal scenes — so the confocal channel-metadata feature was silently inert (output fell back to display-LUT labels like Blue/Gray and a MetadataOnly OME-XML stub). Locate the current scene via reader.metadata.findall(".//Image")[current_scene_index] (aligned with bioio-lif PR #52), keeping reader.scene_root as a plate fast-path. Every fail-safe is preserved: the channel-count-vs-SizeC guard (both directions) and a clean (None, None) decline on any unexpected reader-surface error, so metadata never crashes a conversion. Tests: confocal-path regression cases folded into test_api_lif_wiring (real captured fixture, no bioio_lif import / no large file, CI-safe), with the seam's docstring updated for the new two-tier locator. CHANGELOG: Unreleased entry for the confocal channel-identity feature. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 8 +++ src/zarrmony/api.py | 90 +++++++++++++++++++++----- tests/test_api_lif_wiring.py | 119 ++++++++++++++++++++++++++++++++--- 3 files changed, 193 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66619fa..a53aa88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Leica `.lif` **confocal** channel identity is now preserved in the OME-Zarr + output: each channel's fluorophore (dye), excitation/emission wavelengths, and + detector are read from the scene metadata and written as OME-XML `` + elements and omero channel labels (e.g. `ALEXA 594 (590 nm)`) instead of + display-LUT color names (`Blue`, `Gray`, …). + ## [0.3.6] - 2026-05-15 ### Fixed diff --git a/src/zarrmony/api.py b/src/zarrmony/api.py index 507c93e..552e219 100644 --- a/src/zarrmony/api.py +++ b/src/zarrmony/api.py @@ -203,36 +203,92 @@ def _scene_channel_count(reader: Any) -> int: return int(xarr.sizes["C"]) if "C" in xarr.dims else 1 +def _lif_scene_root_fast(reader: Any) -> ET.Element | None: + """The current scene's settings XML via bioio-lif's ``scene_root`` fast path. + + ``scene_root`` is bioio-lif's *plate* row/column locator: it returns the + well's ```` node for plate scenes but RAISES ``ValueError`` ("Row or + column value is missing…") for ordinary non-plate confocal scenes, and may be + absent on non-LIF readers. We read it inside a guard so either outcome — + raise or ``None`` — is a clean miss, not a crash, letting the caller fall + back to the document-order ```` locator. + """ + try: + return getattr(reader, "scene_root", None) + except Exception: # noqa: BLE001 — a raise here just means "not this path" + return None + + +def _lif_scene_image(reader: Any) -> ET.Element | None: + """The current scene's ```` element from the full LIF XML, fail-safe. + + This is the confocal locator from bioio-lif PR #52: ``reader.metadata`` is + the whole-document LIF ``ElementTree``; ``.//Image`` returns the per-scene + ```` elements in scene order, and ``reader.current_scene_index`` + selects the one being converted. Each such element carries that scene's + ``ChannelDescription`` + ``LDM_Block_Sequential_List`` — exactly what + :func:`extract_channels` consumes. + + Every reader-surface access is guarded so no partially-readable reader can + raise out of here: missing/``None`` metadata, a metadata object without + ``findall``, an empty ```` list, and an absent or out-of-range + ``current_scene_index`` all return ``None`` (a clean miss). Returns the + located element or ``None``; never raises. + """ + metadata = getattr(reader, "metadata", None) + if metadata is None or not hasattr(metadata, "findall"): + return None + images = metadata.findall(".//Image") + if not images: + return None + index = getattr(reader, "current_scene_index", None) + if not isinstance(index, int) or not (0 <= index < len(images)): + return None + return images[index] + + def _lif_scene_channels(reader: Any) -> tuple[list[dict] | None, list[Channel] | None]: """The LIF-vs-other decision, in one place. - Returns ``(extracted, omero_channels)`` when ``reader`` is a LIF reader - (exposes ``scene_root``), its 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)`` 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. + + Locating the scene XML is two-tier (see the two helpers above): + + 1. ``scene_root`` — bioio-lif's plate row/column locator. It works for plate + wells but RAISES for ordinary non-plate **confocal** scenes, so it is only + a *fast path* that preserves existing plate behavior. + 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 (corruption, partial metadata), we decline the projection rather - than label a 2-channel image with 7 names. + 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 an unexpected error — returns + 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. """ try: - # NB: ``scene_root`` is a *property* on bioio_lif.Reader that RAISES - # (ValueError) for ordinary non-plate scenes — the common confocal case — - # so it must be read inside the try, not via getattr() (which only - # swallows AttributeError). A raise here is a clean fall-back, not a crash. - scene_root = getattr(reader, "scene_root", None) - if scene_root is None: + # First-try fast path (plate wells), then the confocal fallback. Each + # locator is internally guarded, so this never raises; an unlocatable + # scene simply yields ``None`` here and a clean decline below. Test + # ``is None`` explicitly rather than ``a or b`` — an ``Element`` with no + # children is falsy, so ``or`` would wrongly skip a valid childless + # ``scene_root`` and is deprecated besides. + scene_element = _lif_scene_root_fast(reader) + if scene_element is None: + scene_element = _lif_scene_image(reader) + if scene_element is None: return None, None - scene_xml = ET.tostring(scene_root, encoding="unicode") + scene_xml = ET.tostring(scene_element, encoding="unicode") extracted = extract_channels(scene_xml) if not extracted or len(extracted) != _scene_channel_count(reader): return None, None diff --git a/tests/test_api_lif_wiring.py b/tests/test_api_lif_wiring.py index dc88118..2fc384d 100644 --- a/tests/test_api_lif_wiring.py +++ b/tests/test_api_lif_wiring.py @@ -1,10 +1,20 @@ -"""LIF api-wiring fail-safe regression. - -The LIF channel-metadata branch must never crash a conversion and must decline -cleanly on: a non-LIF reader, a ``scene_root`` property that *raises* (the common -non-plate confocal case in bioio_lif), or a channel-count vs SizeC mismatch. +"""LIF api-wiring regression — the ``_lif_scene_channels`` scene→identity seam. + +Channel identity is located two-tier (see ``api._lif_scene_root_fast`` / +``_lif_scene_image``): bioio-lif's ``scene_root`` plate-well locator is tried +first, then — because ``scene_root`` *raises* for ordinary non-plate confocal +scenes — the document-order ``.//Image[current_scene_index]`` locator (bioio-lif +PR #52) is the fallback that makes confocal scenes work. These tests pin both that +the confocal path yields real fluorophore identity AND that the seam never crashes +a conversion: it declines cleanly (``(None, None)``) for a non-LIF reader, a reader +exposing neither a usable ``scene_root`` nor ``metadata``, a count-vs-SizeC +mismatch, or any unexpected reader-surface error. + +Driven by faithful reader doubles over the real captured ``lif_confocal_7ch.xml`` +fixture — no ``bioio_lif`` import and no large data file, so it runs in CI. """ +import copy import xml.etree.ElementTree as ET from pathlib import Path @@ -19,8 +29,12 @@ def __init__(self, c): self.sizes = {"C": c} if c is not None else {} +# --- plate / degenerate / non-LIF doubles (the scene_root fast path) ------- + + class _Raising: - """bioio_lif's scene_root raises ValueError for ordinary (non-plate) scenes.""" + """scene_root raises AND no ``metadata`` is exposed — a degenerate reader that + can't be located either way, so it must decline (not crash).""" xarray_dask_data = _Xarr(7) @@ -30,10 +44,14 @@ def scene_root(self): class _NonLif: + """Neither ``scene_root`` nor ``metadata`` — a non-LIF reader.""" + xarray_dask_data = _Xarr(7) class _Good: + """Plate-shaped reader: ``scene_root`` returns the scene ```` directly.""" + xarray_dask_data = _Xarr(7) @property @@ -49,7 +67,61 @@ def scene_root(self): return ET.parse(FIX).getroot() -def test_raising_scene_root_falls_back(): +# --- confocal double (scene_root raises; scenes live in ``metadata``) ------ + + +def _two_scene_metadata() -> ET.Element: + """A LIF metadata tree with two confocal scenes (the second dye-renamed). + + Scene 0 is the real captured 7-channel ````; scene 1 is the same with + its ``DyeName`` values prefixed so correct *positional* indexing is observable. + """ + scene0 = copy.deepcopy(ET.parse(FIX).getroot()) # .. + scene1 = copy.deepcopy(scene0) + for cp in scene1.iter("ChannelProperty"): + key, val = cp.find("Key"), cp.find("Value") + if ( + key is not None + and (key.text or "").strip() == "DyeName" + and val is not None + and val.text + ): + val.text = "B_" + val.text + root = ET.Element("LMSDataContainerHeader") + children = ET.SubElement(ET.SubElement(root, "Element"), "Children") + children.append(scene0) + children.append(scene1) + return root + + +class _ConfocalReader: + """Confocal LIF reader: ``scene_root`` raises; scenes live in ``metadata`` as + the document-order ```` elements (the common confocal case).""" + + def __init__(self, c=7, have_metadata=True): + self._md = _two_scene_metadata() if have_metadata else None + self._c = c + self.current_scene_index = 0 + + @property + def metadata(self): + return self._md + + @property + def scene_root(self): + raise ValueError( + "Row or column value is missing; cannot locate the scene node." + ) + + @property + def xarray_dask_data(self): + return _Xarr(self._c) + + +# --- fail-safe / fall-back paths ------------------------------------------- + + +def test_raising_scene_root_without_metadata_declines(): assert _lif_scene_channels(_Raising()) == (None, None) @@ -61,7 +133,40 @@ def test_count_mismatch_declines(): assert _lif_scene_channels(_CountMismatch()) == (None, None) +# --- plate fast path ------------------------------------------------------- + + def test_happy_path_wiring(): extracted, omero = _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)" + + +# --- confocal (scene_root raises -> metadata .//Image locator) ------------- + + +def test_confocal_scene_yields_fluorophore_channels(): + r = _ConfocalReader() + r.current_scene_index = 0 + 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 + + +def test_confocal_scene_is_located_positionally(): + r = _ConfocalReader() + r.current_scene_index = 1 + _, 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) + + +def test_confocal_no_metadata_falls_back(): + r = _ConfocalReader(have_metadata=False) + assert _lif_scene_channels(r) == (None, None)