From c5f999e6f17435642c8f52d81bb5f07a95ccc5ce Mon Sep 17 00:00:00 2001 From: lorenzo Date: Thu, 12 Nov 2026 14:30:40 +0100 Subject: [PATCH] fix: axes_order only virtualises canonical axes, unknown names raise (#277) --- CHANGELOG.md | 2 + docs/getting_started/2_images.md | 5 +- docs/getting_started/6_iterators.md | 3 +- src/ngio/io_pipes/_ops_axes.py | 37 ++++++++++---- src/ngio/iterators/_feature.py | 3 ++ src/ngio/iterators/_image_processing.py | 3 ++ src/ngio/iterators/_object_detection.py | 3 ++ src/ngio/iterators/_segmentation.py | 5 ++ tests/unit/images/test_omezarr_container.py | 4 +- tests/unit/io_pipes/test_ops_errors.py | 54 +++++++++++++++++++++ tests/unit/iterators/test_iterators.py | 27 ++++++++++- 11 files changed, 132 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa7ac2f7..2702aa1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ - Add `set_config()` (exported from `ngio` and `ngio.config`) to replace the global `NgioConfig` from code at any time. It replaces the previous configuration (a zarr setting forwarded earlier stays unless the new config sets it) and immediately applies the `s3fs` retry markers and the `zarr` section, so libraries can configure ngio without setting `NGIO_CONFIG_PATH` before `import ngio`. +- `axes_order` only virtualises canonical axes. A canonical axis (`t`, `c`, `z`, `y`, `x`) missing on disk still comes back as a singleton, by its canonical name or the name the `AxesSetup` maps to it, so one `"czyx"` keeps working on 2D and 3D data. Any other name that is not an axis of the image raises `NgioValueError` naming it, the full order, the image axes and the canonical set; it used to become a phantom dimension (`axes_order="czyq"` returned a 4D array with `x` dropped and a size-1 axis appended). The iterators run the check in the constructor instead of at the first read, and the duplicate-axis error names the repeated axis ([#277](https://github.com/BioVisionCenter/ngio/issues/277)). + ### Tests - The suite passes under pandas 3.1. The stores test helper no longer calls `set_index(inplace=True)` (deprecated, PDEP-8), and anndata's read of the deprecated `future.infer_string` option is ignored by message in `filterwarnings` until an anndata release carries the upstream fix ([scverse/anndata#2672](https://github.com/scverse/anndata/issues/2672)). The weekly pip CI was red on both jobs ([#266](https://github.com/BioVisionCenter/ngio/issues/266), [#267](https://github.com/BioVisionCenter/ngio/issues/267)). diff --git a/docs/getting_started/2_images.md b/docs/getting_started/2_images.md index 7e428043..62b89092 100644 --- a/docs/getting_started/2_images.md +++ b/docs/getting_started/2_images.md @@ -187,7 +187,10 @@ image.set_array(data) ``` It accepts a numpy array or a dask array, and takes the same slicing and `axes_order` -arguments as the getters, so you can write back exactly the region you read. Writes +arguments as the getters, so you can write back exactly the region you read. In an +`axes_order`, a canonical axis (`t`, `c`, `z`, `y`, `x`) that the image does not have +is a singleton dimension, so one `"czyx"` works on 2D and 3D data; any other name +that is not on disk raises. Writes can also *combine* with what is on disk instead of replacing it — see [Merging instead of overwriting](#merging-instead-of-overwriting) and [Keeping label ids unique across regions](#keeping-label-ids-unique-across-regions) diff --git a/docs/getting_started/6_iterators.md b/docs/getting_started/6_iterators.md index 33126105..8ea5cb2e 100644 --- a/docs/getting_started/6_iterators.md +++ b/docs/getting_started/6_iterators.md @@ -197,7 +197,8 @@ iterator covers the whole image as a single region: The constructors share a small vocabulary of keyword arguments: `channel_selection` restricts the input reads to given channels, `axes_order` -fixes the axes order of the patches the function sees (`"yx"` above), +fixes the axes order of the patches the function sees (`"yx"` above; a canonical +axis the image lacks is a singleton, any other unknown name raises at construction), `input_transforms`/`output_transforms` apply [transforms](../api/ngio/transforms.md) around the function, and the writing iterators take `consolidation_mode` (how the output pyramid is rebuilt at the diff --git a/src/ngio/io_pipes/_ops_axes.py b/src/ngio/io_pipes/_ops_axes.py index 673e9ebd..705a8fb9 100644 --- a/src/ngio/io_pipes/_ops_axes.py +++ b/src/ngio/io_pipes/_ops_axes.py @@ -6,6 +6,7 @@ from pydantic import BaseModel, ConfigDict from ngio.common._dimensions import Dimensions +from ngio.ome_zarr_meta.ngio_specs import canonical_axes_order from ngio.utils import NgioValueError ############################################################## @@ -243,16 +244,17 @@ def set_as_sequence_axes_ops( def _check_output_axes(axes: Sequence[str]) -> None: """Check that the input axes are valid.""" - unique_names = set(axes) - if len(unique_names) != len(axes): - raise NgioValueError( - "Duplicate axis names found. Please provide unique names for each axis." - ) for name in axes: if not isinstance(name, str): raise NgioValueError( f"Invalid axis name '{name}'. Axis names must be strings." ) + duplicates = sorted({name for name in axes if axes.count(name) > 1}) + if duplicates: + raise NgioValueError( + f"Duplicate axis names {duplicates} in axes_order={list(axes)}. " + "Please provide unique names for each axis." + ) def _build_squeeze_tuple( @@ -332,17 +334,34 @@ def _normalize_axes_order( """Convert axes order to the on-disk axes names. In this way there is not unambiguity in the axes order. + + An axis that is not on disk is kept as a virtual (singleton) axis only if + it is a canonical axis, by its canonical name or the name the `AxesSetup` + maps to it. Any other name is an error rather than a phantom dimension. """ + axes_handler = dimensions.axes_handler new_axes_order = [] for axis_name in axes_order: - axis = dimensions.axes_handler.get_axis(axis_name) - if axis is None: - new_axes_order.append(axis_name) - else: + axis = axes_handler.get_axis(axis_name) + if axis is not None: new_axes_order.append(axis.name) + continue + canonical = axes_handler.get_canonical_name(axis_name) or axis_name + if canonical not in canonical_axes_order(): + raise NgioValueError( + f"axes_order={list(axes_order)} contains '{axis_name}', which is " + f"neither an axis of this image {list(axes_handler.axes_names)} " + f"nor a canonical axis {list(canonical_axes_order())}." + ) + new_axes_order.append(canonical) return tuple(new_axes_order) +def validate_axes_order(dimensions: Dimensions, axes_order: Sequence[str]) -> None: + """Fail now, not at the first read, if `axes_order` cannot be honoured.""" + _check_output_axes(_normalize_axes_order(dimensions, axes_order)) + + def build_axes_ops( *, dimensions: Dimensions, diff --git a/src/ngio/iterators/_feature.py b/src/ngio/iterators/_feature.py index d95a3d7e..10472532 100644 --- a/src/ngio/iterators/_feature.py +++ b/src/ngio/iterators/_feature.py @@ -18,6 +18,7 @@ NumpyGetter, TransformProtocol, ) +from ngio.io_pipes._ops_axes import validate_axes_order from ngio.iterators._abstract_iterator import AbstractIteratorBuilder from ngio.iterators._mappers import MapperProtocol from ngio.iterators._partials import ( @@ -328,6 +329,8 @@ def __init__( ) self._channel_selection = channel_selection self._axes_order = axes_order + if axes_order is not None: + validate_axes_order(input_image.dimensions, axes_order) self._input_transforms = input_transforms self._label_transforms = label_transforms diff --git a/src/ngio/iterators/_image_processing.py b/src/ngio/iterators/_image_processing.py index 801a8830..edd7201b 100644 --- a/src/ngio/iterators/_image_processing.py +++ b/src/ngio/iterators/_image_processing.py @@ -20,6 +20,7 @@ ) from ngio.io_pipes._io_pipes_types import DataGetterProtocol, DataSetterProtocol from ngio.io_pipes._merge_policy import MergeInput, resolve_merge +from ngio.io_pipes._ops_axes import validate_axes_order from ngio.iterators._abstract_iterator import OverlapPolicy, WritingIteratorBuilder from ngio.iterators._mappers import MapperProtocol, WriteOrder, validate_write_order @@ -85,6 +86,8 @@ def __init__( self._channel_selection = channel_selection self._output_channel_selection = output_channel_selection self._axes_order = axes_order + if axes_order is not None: + validate_axes_order(input_image.dimensions, axes_order) self._input_transforms = input_transforms self._output_transforms = output_transforms diff --git a/src/ngio/iterators/_object_detection.py b/src/ngio/iterators/_object_detection.py index 9d962df4..7b1ea863 100644 --- a/src/ngio/iterators/_object_detection.py +++ b/src/ngio/iterators/_object_detection.py @@ -33,6 +33,7 @@ add_channel_selection_to_slicing_dict, ) from ngio.io_pipes import DaskGetter, DataGetter, NumpyGetter, TransformProtocol +from ngio.io_pipes._ops_axes import validate_axes_order from ngio.iterators._abstract_iterator import AbstractIteratorBuilder from ngio.iterators._mappers import MapperProtocol from ngio.iterators._partials import ( @@ -418,6 +419,8 @@ def __init__( ) self._channel_selection = channel_selection self._axes_order = axes_order + if axes_order is not None: + validate_axes_order(input_image.dimensions, axes_order) self._input_transforms = input_transforms def _get_init_kwargs(self) -> dict: diff --git a/src/ngio/iterators/_segmentation.py b/src/ngio/iterators/_segmentation.py index f74c35e7..c11012eb 100644 --- a/src/ngio/iterators/_segmentation.py +++ b/src/ngio/iterators/_segmentation.py @@ -23,6 +23,7 @@ from ngio.io_pipes._io_pipes_types import DataGetterProtocol, DataSetterProtocol from ngio.io_pipes._mask_transform import BaseMaskMerge, BaseMaskTransform from ngio.io_pipes._merge_policy import MergeInput, resolve_merge +from ngio.io_pipes._ops_axes import validate_axes_order from ngio.io_pipes._ops_slices_utils import check_if_regions_overlap from ngio.iterators._abstract_iterator import OverlapPolicy, WritingIteratorBuilder from ngio.iterators._mappers import MapperProtocol, WriteOrder, validate_write_order @@ -140,6 +141,8 @@ def __init__( ) self._channel_selection = channel_selection self._axes_order = axes_order + if axes_order is not None: + validate_axes_order(input_image.dimensions, axes_order) self._input_transforms = input_transforms self._output_transforms = output_transforms @@ -607,6 +610,8 @@ def __init__( ) self._channel_selection = channel_selection self._axes_order = axes_order + if axes_order is not None: + validate_axes_order(input_image.dimensions, axes_order) self._input_transforms = input_transforms self._output_transforms = output_transforms diff --git a/tests/unit/images/test_omezarr_container.py b/tests/unit/images/test_omezarr_container.py index 0c9e783e..1d28efeb 100644 --- a/tests/unit/images/test_omezarr_container.py +++ b/tests/unit/images/test_omezarr_container.py @@ -283,9 +283,9 @@ def test_get_and_squeeze(tmp_path: Path): assert image.get_array(channel_selection=(0,), axes_order=None).shape == (1, 20, 30) assert image.get_array(channel_selection=0, axes_order=None).shape == (20, 30) - # Reordering axes and adding a virtual axis + # Reordering axes and adding a virtual (canonical, not on disk) axis assert image.get_array( - channel_selection=0, axes_order=["c", "x", "y", "virtual"] + channel_selection=0, axes_order=["c", "x", "y", "z"] ).shape == ( 1, 30, diff --git a/tests/unit/io_pipes/test_ops_errors.py b/tests/unit/io_pipes/test_ops_errors.py index 6070b527..2bd83ef1 100644 --- a/tests/unit/io_pipes/test_ops_errors.py +++ b/tests/unit/io_pipes/test_ops_errors.py @@ -5,6 +5,7 @@ from ngio import Roi, create_empty_ome_zarr from ngio.common import Dimensions from ngio.io_pipes._match_shape import numpy_match_shape +from ngio.io_pipes._ops_axes import build_axes_ops, validate_axes_order from ngio.io_pipes._ops_slices import build_slicing_ops from ngio.io_pipes._zoom_transform import BaseZoomTransform from ngio.ome_zarr_meta import AxesHandler, Dataset @@ -266,3 +267,56 @@ def test_negative_value_in_slicing_sequence_raises_ngio_error(): dimensions=dims, slicing_dict={"y": [-1, 0]}, ) + + +############################################################## +# _ops_axes.py: which names may be virtual axes (#277) +############################################################## + + +def test_axes_order_canonical_axis_missing_on_disk_is_virtual(): + dims = _make_dims(["c", "y", "x"], (2, 8, 8)) + ops = build_axes_ops( + dimensions=dims, input_axes=("c", "y", "x"), axes_order=["c", "z", "y", "x"] + ) + assert ops.output_axes == ("c", "z", "y", "x") + assert ops.expand_op == (1,) + + +def test_axes_order_unknown_name_raises(): + dims = _make_dims(["c", "y", "x"], (2, 8, 8)) + with pytest.raises(NgioValueError, match="'typo'") as exc: + build_axes_ops( + dimensions=dims, + input_axes=("c", "y", "x"), + axes_order=["c", "x", "y", "typo"], + ) + # the message echoes the order, the image axes and the canonical set + assert "['c', 'x', 'y', 'typo']" in str(exc.value) + assert "['c', 'y', 'x']" in str(exc.value) + assert "['t', 'c', 'z', 'y', 'x']" in str(exc.value) + + +def test_axes_order_setup_alias_for_missing_canonical_axis_is_virtual(): + # the setup maps "channel" to "c"; the image has no channel axis, so + # asking for "channel" is a virtual "c" axis, not an unknown name + dims = _make_dims(["y", "x"], (8, 8), axes_setup=AxesSetup(c="channel")) + ops = build_axes_ops( + dimensions=dims, input_axes=("y", "x"), axes_order=["channel", "y", "x"] + ) + assert ops.output_axes == ("c", "y", "x") + + +def test_axes_order_duplicate_names_are_named(): + dims = _make_dims(["c", "y", "x"], (2, 8, 8)) + with pytest.raises(NgioValueError, match=r"Duplicate axis names \['y'\]"): + build_axes_ops( + dimensions=dims, input_axes=("c", "y", "x"), axes_order=["c", "y", "y"] + ) + + +def test_validate_axes_order_accepts_string_orders(): + dims = _make_dims(["c", "y", "x"], (2, 8, 8)) + validate_axes_order(dims, "zyx") + with pytest.raises(NgioValueError, match="'q'"): + validate_axes_order(dims, "czyq") diff --git a/tests/unit/iterators/test_iterators.py b/tests/unit/iterators/test_iterators.py index 1469dfe2..6a26d159 100644 --- a/tests/unit/iterators/test_iterators.py +++ b/tests/unit/iterators/test_iterators.py @@ -5,7 +5,11 @@ import pytest from zarr.storage import MemoryStore -from ngio import create_ome_zarr_from_array, open_ome_zarr_container +from ngio import ( + create_empty_ome_zarr, + create_ome_zarr_from_array, + open_ome_zarr_container, +) from ngio.iterators import ( FeatureExtractorIterator, ImageProcessingIterator, @@ -356,3 +360,24 @@ def __init__(self, input_image, output_label, *, favourite=None, **kwargs): ) with pytest.raises(NgioValueError, match="favourite"): iterator.by_yx() + + +def test_iterator_rejects_unknown_axes_order_at_construction(): + # a typo in `axes_order` fails in the constructor, not at the first read (#277) + ome_zarr = create_empty_ome_zarr( + MemoryStore(), + shape=(2, 8, 8), + axes_names=("c", "y", "x"), + pixelsize=0.5, + levels=2, + ) + image = ome_zarr.get_image() + t_image = ome_zarr.derive_image(store=MemoryStore()).get_image() + label = ome_zarr.derive_label("label") + + with pytest.raises(NgioValueError, match="'typo'"): + ImageProcessingIterator(image, t_image, axes_order=["c", "x", "y", "typo"]) + with pytest.raises(NgioValueError, match="'q'"): + SegmentationIterator(image, label, channel_selection=0, axes_order="yxq") + # a canonical axis missing on disk stays a virtual axis + ImageProcessingIterator(image, t_image, axes_order="czyx")