From d88ea841f2535a0bbdfeb900331a5fed80e266ef Mon Sep 17 00:00:00 2001 From: Max Ferrin Date: Thu, 23 Jul 2026 14:37:42 -0700 Subject: [PATCH] fix(omero): default channel window to array dtype range, not 0-255 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OME-Zarr `omero.channels[i].window` block was hardcoded to `{min: 0, max: 255, start: 0, end: 255}` on every store because zarrmony constructed `bioio_ome_zarr.writers.Channel(...)` without passing `window=`. Every uint16 / uint32 / float32 store therefore shipped with a display window clamped to 8-bit range and appeared black on first open in napari and OMERO — the acquisition's real intensities lived far outside the window, so viewers rendered everything above 255 as saturated white and everything else as noise near zero. Fix: derive the window from the reader's dtype at the three `Channel(...)` construction sites (`api._lif_scene_channels`, `api._channels_for_scene`, `writers.scene._default_channels`). Integer dtypes use `np.iinfo(min, max)`; float dtypes use `0.0`/`1.0` (OMERO convention for normalized floats). `start` / `end` mirror `min` / `max` so first-open viewers see the full range unclipped — percentile-based auto-contrast is a separate concern, not this bug. Closes #50. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 13 ++++++ src/zarrmony/api.py | 9 ++-- src/zarrmony/writers/scene.py | 29 ++++++++++-- tests/conftest.py | 18 +++++++- tests/test_api.py | 39 ++++++++++++++++ tests/test_api_lif_wiring.py | 12 +++++ tests/test_lif_projections.py | 38 ++++++++++++++++ tests/test_scene_writer.py | 83 ++++++++++++++++++++++++++++++++++- 8 files changed, 232 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b731af..f53e469 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- OMERO display window (`omero.channels[i].window`) now spans the array's + dtype range instead of the hardcoded 0–255. Both `Channel(...)` construction + sites (`api._lif_scene_channels` and `writers.scene._default_channels`, plus + the shared `api._channels_for_scene` name-based path) pass `window=` derived + from the reader dtype: integer dtypes use `np.iinfo(min, max)` and float + dtypes use `0.0`/`1.0` (OMERO convention for normalized floats). uint16 / + uint32 / float32 stores no longer open black in napari and OMERO because the + display window was clamping intensities into an 8-bit band. `start` / `end` + mirror `min` / `max` — percentile-based auto-contrast is out of scope for + this fix. (#50) + ## [0.8.0] - 2026-07-10 ### Added diff --git a/src/zarrmony/api.py b/src/zarrmony/api.py index eafbc8c..0e45568 100644 --- a/src/zarrmony/api.py +++ b/src/zarrmony/api.py @@ -54,7 +54,7 @@ ) from zarrmony.writers.per_scene import write_per_scene_metadata from zarrmony.writers.plate import summarize_plate_layout, write_plate -from zarrmony.writers.scene import write_scene +from zarrmony.writers.scene import _dtype_window, write_scene Layout = Literal["auto", "per-scene", "bf2raw", "plate"] ResolvedLayout = Literal["per-scene", "bf2raw", "plate"] @@ -206,8 +206,10 @@ def _channels_for_scene( if not channel_names: return None colors = colors_for_channels(channel_names, overrides=channel_colors) + window = _dtype_window(reader.dtype) return [ - Channel(label=n, color=c) for n, c in zip(channel_names, colors, strict=True) + Channel(label=n, color=c, window=window) + for n, c in zip(channel_names, colors, strict=True) ] @@ -268,8 +270,9 @@ def _lif_scene_channels(reader: Any) -> tuple[list[dict] | None, list[Channel] | extracted = extract_channels(scene_xml) if not extracted or len(extracted) != _scene_channel_count(reader): return None, None + window = _dtype_window(reader.dtype) omero_channels = [ - Channel(label=o["label"], color=o["color"]) + Channel(label=o["label"], color=o["color"], window=window) for o in channels_to_omero(extracted) ] return extracted, omero_channels diff --git a/src/zarrmony/writers/scene.py b/src/zarrmony/writers/scene.py index 1e7b60b..186eb99 100644 --- a/src/zarrmony/writers/scene.py +++ b/src/zarrmony/writers/scene.py @@ -10,6 +10,7 @@ from typing import Any import dask.array as da +import numpy as np import xarray as xr from bioio_ome_zarr.writers import Channel, OMEZarrWriter @@ -59,8 +60,30 @@ def _physical_scales_for_dims(dims: Sequence[str], reader: Any) -> list[float]: return [base[d] for d in dims] -def _default_channels(channel_names: Sequence[str]) -> list[Channel]: - return [Channel(label=name, color="ffffff") for name in channel_names] +def _dtype_window(dtype: np.dtype) -> dict[str, int | float]: + """OMERO display-window bounds spanning ``dtype``'s full range. + + Integer dtypes → ``np.iinfo(dtype).min`` / ``max``; float dtypes → ``0.0`` / + ``1.0`` (OMERO convention for normalized floats). ``start`` / ``end`` mirror + ``min`` / ``max`` so first-open viewers see the full range unclipped — + percentile-based auto-contrast is a separate concern (see #50). Prevents + the bioio-ome-zarr ``Channel`` default of ``0``–``255`` from clamping + uint16/uint32/float32 pixels into a black-on-first-open display. + """ + if np.issubdtype(dtype, np.integer): + info = np.iinfo(dtype) + lo: int | float = int(info.min) + hi: int | float = int(info.max) + else: + lo, hi = 0.0, 1.0 + return {"min": lo, "max": hi, "start": lo, "end": hi} + + +def _default_channels(channel_names: Sequence[str], dtype: np.dtype) -> list[Channel]: + window = _dtype_window(dtype) + return [ + Channel(label=name, color="ffffff", window=window) for name in channel_names + ] def write_scene( @@ -112,7 +135,7 @@ def write_scene( channel_names = [f"C:{i}" for i in range(canonical.sizes["C"])] else: channel_names = [] - channels = _default_channels(channel_names) + channels = _default_channels(channel_names, canonical.dtype) channel_count = len(channels) axes_names = [d.lower() for d in dims] diff --git a/tests/conftest.py b/tests/conftest.py index 2026dad..14ef8e0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -122,6 +122,7 @@ def __init__( mosaic_summary: dict | None = None, skip_reasons: dict[int, str] | None = None, per_tile_scenes: dict[int, "TileScene"] | None = None, + dtype: np.typing.DTypeLike = np.uint16, ) -> None: self.scenes = tuple(scenes) self._dims = dims @@ -136,6 +137,7 @@ def __init__( self.mosaic_summary = mosaic_summary self._skip_reasons = skip_reasons or {} self._per_tile_scenes = per_tile_scenes or {} + self._dtype = np.dtype(dtype) @property def skip_reason(self) -> str | None: @@ -145,6 +147,16 @@ def skip_reason(self) -> str | None: def channel_names(self) -> list[str]: return self._channel_names + @property + def dtype(self) -> np.dtype: + """Mirrors bioio's ``Reader.dtype`` — the pixel dtype of the current scene. + + Held as a scalar attr so tests can vary it without allocating the full + xarray. The ``xarray_dask_data`` property honors this dtype when + materializing the fake pixel buffer. + """ + return self._dtype + def set_scene(self, idx: int | str) -> None: if isinstance(idx, str): idx = self.scenes.index(idx) @@ -161,7 +173,9 @@ def is_mosaic_reassembly_eligible(self) -> bool: @property def xarray_dask_data(self) -> xr.DataArray: # Fill with scene-index+1 so tests can assert "the right scene was read" - arr = np.full(self._shape, fill_value=self._current_scene + 1, dtype=np.uint16) + arr = np.full( + self._shape, fill_value=self._current_scene + 1, dtype=self._dtype + ) coords: dict[str, list[str]] = {} if "C" in self._dims and self._channel_names: coords["C"] = self._channel_names @@ -185,7 +199,7 @@ def tiles_xarray_dask_data(self) -> xr.DataArray: scene.tile_yx[0], scene.tile_yx[1], ) - arr = np.zeros(shape, dtype=np.uint16) + arr = np.zeros(shape, dtype=self._dtype) for i in range(m): arr[i].fill(i + 1) return xr.DataArray(da.from_array(arr), dims=["M", "T", "C", "Z", "Y", "X"]) diff --git a/tests/test_api.py b/tests/test_api.py index 88e751a..d4641c0 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -8,6 +8,7 @@ import warnings from pathlib import Path +import numpy as np import pytest import zarr from ome_types import from_xml @@ -212,6 +213,44 @@ def test_per_scene_sanitizes_scene_names_in_dirnames( assert set(by_name) == {"a/b", "c d"} +# ---------- omero display window matches array dtype (#50) ---------- + + +@pytest.mark.parametrize( + "dtype, expected_window", + [ + (np.uint8, {"min": 0, "max": 255, "start": 0, "end": 255}), + (np.uint16, {"min": 0, "max": 65535, "start": 0, "end": 65535}), + (np.float32, {"min": 0.0, "max": 1.0, "start": 0.0, "end": 1.0}), + ], +) +def test_per_scene_omero_window_matches_reader_dtype( + tmp_path: Path, patched_reader, dtype, expected_window +) -> None: + """The ``_channels_for_scene`` api path (named channels, non-LIF) must ship + an OMERO display window spanning the reader's dtype range — not the + bioio-ome-zarr ``Channel`` default of 0–255 that would make uint16 stores + open black. Regression guard for #50. + """ + reader = FakeReader( + scenes=["s"], + dims="TCYX", + shape=(1, 2, 32, 32), + channel_names=["DAPI", "GFP"], + dtype=dtype, + ) + patched_reader(reader) + out = tmp_path / "out" + + convert("/tmp/x.czi", out, pyramid_min_size=8) + + g = zarr.open_group(str(out / "s.ome.zarr"), mode="r") + channels = g.attrs["ome"]["omero"]["channels"] + assert len(channels) == 2 + for c in channels: + assert c["window"] == expected_window + + # ---------- bf2raw mode (opt-in) ---------- diff --git a/tests/test_api_lif_wiring.py b/tests/test_api_lif_wiring.py index 2fc384d..34a12a8 100644 --- a/tests/test_api_lif_wiring.py +++ b/tests/test_api_lif_wiring.py @@ -18,10 +18,17 @@ import xml.etree.ElementTree as ET from pathlib import Path +import numpy as np + from zarrmony.api import _lif_scene_channels FIX = Path(__file__).parent / "fixtures" / "lif_confocal_7ch.xml" +# Every double below mirrors bioio's ``Reader.dtype`` surface (uint16 is the +# LIF acquisition default) — ``_lif_scene_channels`` reads it to size the +# OMERO display window per array-dtype range (issue #50). +_DTYPE = np.dtype(np.uint16) + class _Xarr: def __init__(self, c): @@ -37,6 +44,7 @@ class _Raising: can't be located either way, so it must decline (not crash).""" xarray_dask_data = _Xarr(7) + dtype = _DTYPE @property def scene_root(self): @@ -47,12 +55,14 @@ class _NonLif: """Neither ``scene_root`` nor ``metadata`` — a non-LIF reader.""" xarray_dask_data = _Xarr(7) + dtype = _DTYPE class _Good: """Plate-shaped reader: ``scene_root`` returns the scene ```` directly.""" xarray_dask_data = _Xarr(7) + dtype = _DTYPE @property def scene_root(self): @@ -61,6 +71,7 @@ def scene_root(self): class _CountMismatch: xarray_dask_data = _Xarr(2) + dtype = _DTYPE @property def scene_root(self): @@ -102,6 +113,7 @@ 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 + self.dtype = _DTYPE @property def metadata(self): diff --git a/tests/test_lif_projections.py b/tests/test_lif_projections.py index fb1fad7..9acdaa9 100644 --- a/tests/test_lif_projections.py +++ b/tests/test_lif_projections.py @@ -14,6 +14,8 @@ import xml.etree.ElementTree as ET from pathlib import Path +import numpy as np +import pytest import zarr from ome_types import from_xml @@ -261,6 +263,42 @@ def test_api_lif_channel_count_mismatch_falls_back(tmp_path: Path, monkeypatch) assert len(g.attrs["ome"]["omero"]["channels"]) == 2 +@pytest.mark.parametrize( + "dtype, expected_window", + [ + (np.uint8, {"min": 0, "max": 255, "start": 0, "end": 255}), + (np.uint16, {"min": 0, "max": 65535, "start": 0, "end": 65535}), + (np.float32, {"min": 0.0, "max": 1.0, "start": 0.0, "end": 1.0}), + ], +) +def test_api_lif_omero_window_matches_reader_dtype( + tmp_path: Path, monkeypatch, dtype, expected_window +) -> None: + """The LIF ``_lif_scene_channels`` path must also honor the reader's dtype + when constructing the OMERO display window. Same regression as #50 but for + the LIF-identity branch that would otherwise ship 0–255 next to real + fluorophore labels. + """ + xml = FIXTURE.read_text(encoding="utf-8") + reader = FakeLifReader( + scene_xml=xml, + scenes=["scene0"], + dims="CYX", + shape=(7, 16, 16), + dtype=dtype, + ) + _install(monkeypatch, reader) + out = tmp_path / "out" + + convert("/tmp/x.lif", out, pyramid_min_size=8) + + g = zarr.open_group(str(out / "scene0.ome.zarr"), mode="r") + channels = g.attrs["ome"]["omero"]["channels"] + assert len(channels) == 7 + for c in channels: + assert c["window"] == expected_window + + def test_api_lif_with_garbage_scene_root_falls_back( tmp_path: Path, monkeypatch ) -> None: diff --git a/tests/test_scene_writer.py b/tests/test_scene_writer.py index bd14d5c..629bdf6 100644 --- a/tests/test_scene_writer.py +++ b/tests/test_scene_writer.py @@ -1,9 +1,11 @@ """Integration tests for write_scene using FakeReader from conftest.""" +import numpy as np +import pytest import zarr from tests.conftest import FakePhysicalPixelSizes, FakeReader -from zarrmony.writers.scene import write_scene +from zarrmony.writers.scene import _dtype_window, write_scene def test_write_scene_writes_pyramid_and_metadata(tmp_path) -> None: @@ -78,6 +80,85 @@ def test_write_scene_returns_audit_record(tmp_path) -> None: assert "mosaic" not in audit +def test_dtype_window_uint8_spans_full_byte_range() -> None: + assert _dtype_window(np.dtype(np.uint8)) == { + "min": 0, + "max": 255, + "start": 0, + "end": 255, + } + + +def test_dtype_window_uint16_spans_full_16bit_range() -> None: + assert _dtype_window(np.dtype(np.uint16)) == { + "min": 0, + "max": 65535, + "start": 0, + "end": 65535, + } + + +def test_dtype_window_uint32_spans_full_32bit_range() -> None: + w = _dtype_window(np.dtype(np.uint32)) + assert w == {"min": 0, "max": 4294967295, "start": 0, "end": 4294967295} + + +def test_dtype_window_int16_covers_signed_range() -> None: + assert _dtype_window(np.dtype(np.int16)) == { + "min": -32768, + "max": 32767, + "start": -32768, + "end": 32767, + } + + +def test_dtype_window_float32_uses_normalized_range() -> None: + # OMERO convention for normalized floats: 0.0/1.0, not the dtype extrema + # (finfo.min/max would be ±3.4e38 and viewers can't render that). + assert _dtype_window(np.dtype(np.float32)) == { + "min": 0.0, + "max": 1.0, + "start": 0.0, + "end": 1.0, + } + + +def test_dtype_window_float64_uses_normalized_range() -> None: + assert _dtype_window(np.dtype(np.float64)) == { + "min": 0.0, + "max": 1.0, + "start": 0.0, + "end": 1.0, + } + + +@pytest.mark.parametrize( + "dtype, expected_window", + [ + (np.uint8, {"min": 0, "max": 255, "start": 0, "end": 255}), + (np.uint16, {"min": 0, "max": 65535, "start": 0, "end": 65535}), + (np.float32, {"min": 0.0, "max": 1.0, "start": 0.0, "end": 1.0}), + ], +) +def test_write_scene_omero_window_matches_array_dtype( + tmp_path, dtype, expected_window +) -> None: + """Regression for #50 — the default-path OMERO window must span the array's + dtype range, not the bioio-ome-zarr ``Channel`` fallback of 0–255. Otherwise + every uint16/uint32/float32 store appears black-on-first-open in napari and + OMERO because the display window clamps intensities into an 8-bit band. + """ + reader = FakeReader(scenes=["s"], dims="TCYX", shape=(1, 1, 32, 32), dtype=dtype) + out = tmp_path / f"{np.dtype(dtype).name}.zarr" + + write_scene(reader, scene_index=0, store_path=str(out), pyramid_min_size=8) + + g = zarr.open_group(str(out), mode="r") + channels = g.attrs["ome"]["omero"]["channels"] + assert len(channels) == 1 + assert channels[0]["window"] == expected_window + + def test_write_scene_records_mosaic_summary(tmp_path) -> None: mosaic = {"stitched": True, "tile_count": 12, "tile_shape": {"Y": 5048, "X": 5048}} reader = FakeReader(