fix minor bug when setting start/and window settings using percentile

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## [v1.1.3]
+
+### Fixes
+
+- Channel windows could land on disk with `start == end`. `compute_image_percentile` returned a degenerate window for a constant, binary, sparse or empty channel: the percentile window now opens to the channel max when the percentiles coincide, then to `(0, value)` for a constant channel (a binary mask gets `(0, 1)`), and an empty channel gets `(0, 1)`. Such a window is spec-legal but useless to a viewer, and napari refuses to open it ([#74](https://github.com/BioVisionCenter/ngio/issues/74)).
+- `ChannelVisualisation` no longer silently rewrites `end` to `start + 1` when the two are equal. The repair ran at model construction, so it never reached the disk through `set_channel_windows` (built with `model_copy`, which skips validators) while a file that already carried `(7, 7)` read back as `(7, 8)`: ngio reported a window the disk did not hold. The model now reflects the disk. Instead, `set_channel_windows` warns (`NgioUserWarning`) when a `start`/`end` or `min`/`max` pair is not strictly increasing and writes the values as given, since the spec puts no order on them.
+
 ## [v1.1.2]
 
 ### Fixes

src/ngio/images/_image.py

@@ -1,6 +1,7 @@
 """Generic class to handle Image-like data in a OME-NGFF file."""
 
 import logging
+import warnings
 from collections.abc import Mapping, Sequence
 from typing import Any, Literal
 
@@ -37,6 +38,7 @@
 from ngio.ome_zarr_meta.ngio_specs import (
     Channel,
     ChannelsMeta,
+    ChannelVisualisation,
     DefaultSpaceUnit,
     DefaultTimeUnit,
     NgffVersions,
@@ -45,6 +47,7 @@
 )
 from ngio.ome_zarr_meta.ngio_specs._axes import AxesSetup
 from ngio.utils import (
+    NgioUserWarning,
     NgioValueError,
     StoreOrGroup,
     ZarrGroupHandler,
@@ -689,6 +692,10 @@ def set_channel_windows(
         These values are used by viewers to set the display
         range of each channel.
 
+        The spec puts no order on `start`/`end` or `min`/`max`, so any value is
+        written as given, but a pair that is not strictly increasing is useless
+        to a viewer and napari refuses it, so it warns.
+
         Args:
             starts_ends (Sequence[tuple[float, float]]): The start and end values
                 for each channel.
@@ -713,10 +720,16 @@ def set_channel_windows(
         # opens the image and reloads its metadata, and it is the same value
         # already bound above.
         for se, mm, ch in zip(starts_ends, min_max_, current_channels, strict=True):
+            _warn_if_not_increasing("start/end", ch.label, se)
             updates = {"start": se[0], "end": se[1]}
             if mm is not None:
+                _warn_if_not_increasing("min/max", ch.label, mm)
                 updates.update({"min": mm[0], "max": mm[1]})
-            channel_visualisation = ch.channel_visualisation.model_copy(update=updates)
+            # `model_validate`, not `model_copy(update=...)`: a copy skips the
+            # validators, so what lands on disk would never be checked.
+            channel_visualisation = ChannelVisualisation.model_validate(
+                {**ch.channel_visualisation.model_dump(), **updates}
+            )
             channel = ch.model_copy(
                 update={"channel_visualisation": channel_visualisation}
             )
@@ -919,6 +932,24 @@ def get(
         )
 
 
+def _warn_if_not_increasing(what: str, label: str, pair: tuple[float, float]) -> None:
+    """Warn when a channel window pair is not strictly increasing.
+
+    The OME-NGFF spec puts no order on the pair, so it is written as given;
+    but it is useless to a viewer and napari refuses to open it.
+    """
+    low, high = pair
+    if high > low:
+        return
+    warnings.warn(
+        f"Channel {label!r}: {what} = ({low}, {high}) is not strictly "
+        "increasing. The spec allows it and the value is written as given, but "
+        "viewers cannot use it and napari refuses to open it.",
+        NgioUserWarning,
+        stacklevel=3,
+    )
+
+
 def compute_image_percentile(
     image: Image,
     percentiles: tuple[float, float] | list[tuple[float, float]] = (0.1, 99.9),
@@ -933,6 +964,11 @@ def compute_image_percentile(
 
     Returns:
         A tuple containing the start and end percentiles for each channel.
+
+    Note:
+        Viewers need `start < end`. When the percentiles coincide (sparse or
+        constant data) the window opens to the channel max, then to `0`; an
+        empty channel gets `(0, 1)`.
     """
     num_channels = image.num_channels
     # handle the case where a single tuple is provided
@@ -967,15 +1003,22 @@ def compute_image_percentile(
         data = data[mask]
         _data = data.compute()
         if _data.size == 0:
-            starts_and_ends.append((0.0, 0.0))
+            starts_and_ends.append((0.0, 1.0))
             continue
 
         # compute the percentiles
         _s_perc, _e_perc = da.percentile(
             data, [start_percentile, end_percentile], method="nearest"
         ).compute()
-
-        starts_and_ends.append((float(_s_perc), float(_e_perc)))
+        start, end = float(_s_perc), float(_e_perc)
+        if end <= start:
+            # Sparse data: the nearest percentiles coincide. Open to the max.
+            end = float(_data.max())
+        if end <= start:
+            # Constant channel: show the value against the zero background.
+            # Every masked value is > 0, so this is always a valid window.
+            start = 0.0
+        starts_and_ends.append((start, end))
     return starts_and_ends
 
 

src/ngio/ome_zarr_meta/ngio_specs/_channels.py

@@ -9,7 +9,7 @@
 from typing import Any, TypeVar
 
 import numpy as np
-from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
+from pydantic import BaseModel, ConfigDict, Field, field_validator
 
 from ngio.utils import NgioValidationError, NgioValueError
 
@@ -156,20 +156,6 @@ def validate_color(cls, value: str | NgioColors) -> str:
         else:
             raise NgioValueError(f"Invalid color {value}.")
 
-    @model_validator(mode="before")
-    def check_start_end(cls, data):
-        """Check that the start and end values are valid.
-
-        If the start and end values are equal, set the end value to start + 1
-        """
-        start = data.get("start", None)
-        end = data.get("end", None)
-        if start is None or end is None:
-            return data
-        if abs(end - start) < 1e-6:
-            data["end"] = start + 1
-        return data
-
     @classmethod
     def default_init(
         cls,

tests/unit/images/test_image_api.py

@@ -1,5 +1,7 @@
 """Coverage tests for channel metadata and channel selection in ngio images."""
 
+from typing import Any
+
 import dask.array as da
 import numpy as np
 import pytest
@@ -9,7 +11,7 @@
 from ngio import create_empty_ome_zarr
 from ngio.images import ChannelSelectionModel
 from ngio.ome_zarr_meta.ngio_specs import Channel, ChannelsMeta
-from ngio.utils import NgioValueError
+from ngio.utils import NgioUserWarning, NgioValueError
 
 
 def _make_container():
@@ -172,3 +174,86 @@ def test_resolve_channel_selection():
         image.resolve_channel_selection("not_a_channel")
     with pytest.raises(NgioValueError):
         image.resolve_channel_selection(["DAPI", 7])
+
+
+def _raw_windows(store) -> list[tuple[float, float]]:
+    """Read the channel windows from the on-disk JSON, bypassing ngio's models."""
+    import zarr
+
+    attrs: dict[str, Any] = dict(zarr.open_group(store, mode="r").attrs)
+    channels = attrs["omero"]["channels"]
+    return [(c["window"]["start"], c["window"]["end"]) for c in channels]
+
+
+@pytest.mark.parametrize(
+    ("data", "expected"),
+    [
+        # empty: no nonzero data at all
+        (np.zeros((64, 64), dtype="uint16"), (0.0, 1.0)),
+        # constant: percentiles and max coincide, drop the start to 0
+        (np.full((64, 64), 7, dtype="uint16"), (0.0, 7.0)),
+        # binary mask: the nonzero data is constant
+        (np.indices((64, 64)).sum(axis=0) % 2, (0.0, 1.0)),
+        # float constant
+        (np.full((64, 64), 0.3, dtype="float32"), (0.0, np.float32(0.3))),
+    ],
+)
+def test_percentile_window_never_degenerate(data, expected):
+    """Viewers need start < end; the raw JSON must never carry start == end."""
+    store = MemoryStore()
+    ome_zarr = create_empty_ome_zarr(
+        store,
+        shape=(1, 64, 64),
+        axes_names=("c", "y", "x"),
+        pixelsize=1.0,
+        levels=1,
+        dtype=str(data.dtype),
+    )
+    ome_zarr.get_image().set_array(data[None])
+    ome_zarr.set_channel_windows_with_percentiles()
+    (window,) = _raw_windows(store)
+    assert window[0] < window[1]
+    assert window == pytest.approx(expected)
+
+
+def test_percentile_window_sparse_opens_to_max():
+    """A single outlier is beyond the 99.9th percentile: the window opens to it."""
+    data = np.ones((1, 64, 64), dtype="uint8")
+    data[0, 3, 5] = 2
+    store = MemoryStore()
+    ome_zarr = create_empty_ome_zarr(
+        store,
+        shape=data.shape,
+        axes_names=("c", "y", "x"),
+        pixelsize=1.0,
+        levels=1,
+        dtype="uint8",
+    )
+    ome_zarr.get_image().set_array(data)
+    ome_zarr.set_channel_windows_with_percentiles()
+    assert _raw_windows(store) == [(1.0, 2.0)]
+
+
+def test_set_channel_windows_warns_but_writes_as_given():
+    """A non-increasing pair is spec-legal: warn, but write exactly what was asked."""
+    store = MemoryStore()
+    ome_zarr = create_empty_ome_zarr(
+        store,
+        shape=(2, 8, 8),
+        axes_names=("c", "y", "x"),
+        pixelsize=1.0,
+        levels=1,
+    )
+    with pytest.warns(NgioUserWarning, match="start/end = \\(5.0, 5.0\\)"):
+        ome_zarr.set_channel_windows(starts_ends=[(5.0, 5.0), (0.0, 1.0)])
+    with pytest.warns(NgioUserWarning, match="start/end = \\(3.0, 1.0\\)"):
+        ome_zarr.set_channel_windows(starts_ends=[(0.0, 1.0), (3.0, 1.0)])
+    assert _raw_windows(store) == [(0.0, 1.0), (3.0, 1.0)]
+    with pytest.warns(NgioUserWarning, match="min/max = \\(7, 7\\)"):
+        ome_zarr.set_channel_windows(
+            starts_ends=[(0.0, 1.0), (0.0, 1.0)], min_max=[(7, 7), (0, 255)]
+        )
+    # The model reflects the disk: no silent repair on read either.
+    (bad, good) = ome_zarr.get_image().channels_meta.channels
+    assert (bad.channel_visualisation.min, bad.channel_visualisation.max) == (7, 7)
+    assert (good.channel_visualisation.min, good.channel_visualisation.max) == (0, 255)