Slicer: normalize comma-separated selections — dedup always, axis order by default

src/ezmsg/sigproc/slicer.py

@@ -70,7 +70,10 @@ def parse_slice(
     - "{start}:{stop}" or {start}:{stop}:{step} -> slice(start, stop, step)
     - "5" (or any integer) -> (5,). Take only that item.
         applying this to a ndarray or AxisArray will drop the dimension.
-    - A comma-separated list of the above -> a tuple of slices | ints
+    - A comma-separated list of the above -> a tuple of slices | ints. Per-token
+      results are concatenated in token order, duplicates included; note that
+      :obj:`SlicerTransformer` normalizes the resolved indices per its ``order``
+      setting (deduplicated, and by default sorted into axis order).
     - A comma-separated list of values and axinfo is provided and is a CoordinateAxis -> a tuple of ints.
       Each value is first compared against the axis labels for an exact match; failing
       that, it is treated as a regular expression and full-matched against the labels
@@ -137,7 +140,9 @@ class SlicerSettings(ez.Settings):
     """selection: See :obj:`ezmsg.sigproc.slicer.parse_slice` for details.
     Label/regex selections always preserve the sliced axis — a single matching
     entry yields a length-1 axis. Only a bare-integer positional selection
-    (e.g. "5") drops the dimension."""
+    (e.g. "5") drops the dimension. Comma-separated selections are normalized
+    per ``order``: duplicates from overlapping tokens are always removed, and
+    by default the result follows axis order regardless of token order."""
 
     axis: str | None = None
     """The name of the axis to slice along. If None, the last axis is used."""
@@ -149,6 +154,22 @@ class SlicerSettings(ez.Settings):
     no longer positional indices (use slice syntax like "3:4" for positions) — and
     raises an error if the axis has no such field."""
 
+    order: str = "axis"
+    """How to order the entries a comma-separated selection resolves to.
+
+    - "axis" (default): the resolved indices are deduplicated and sorted into
+      axis order — a selection is a *filter*, so selections naming the same
+      entries in a different token order (e.g. ``".*-aip-.*,.*-m1-.*"`` vs
+      ``".*-m1-.*,.*-aip-.*"``) produce identical output. An info message is
+      logged when this reorders relative to token order.
+    - "selection": entries follow token order, so a positional selection like
+      "3,1,2" is an intentional permutation.
+
+    In both modes duplicate indices from overlapping tokens (e.g. "0:3,1") are
+    removed — first occurrence wins — with a warning, so a coordinate axis of
+    unique labels stays unique. A single non-comma slice token (e.g. "::-1") is
+    applied as-is and is not normalized."""
+
     on_empty: str = "warn"
     """What to do when a label/regex selection matches nothing on the target axis.
 
@@ -190,9 +211,38 @@ def _selects_positional_int(self, axinfo: AxisArray.CoordinateAxis | None) -> bo
             return False
         return True
 
+    def _normalize_indices(self, indices: npt.NDArray, axis: str) -> npt.NDArray:
+        """Deduplicate the resolved indices and, with order="axis", sort them into
+        axis order, logging when normalization changes anything. Keeps the
+        coordinate axis a set of unique entries even when selection tokens overlap,
+        and (by default) makes the output independent of token order."""
+        _, first_pos = np.unique(indices, return_index=True)
+        deduped = indices[np.sort(first_pos)]
+        if len(deduped) < len(indices):
+            ez.logger.warning(
+                "Slicer: selection %r has overlapping tokens on axis %r; removed "
+                "%d duplicate(s), keeping the first occurrence.",
+                self.settings.selection,
+                axis,
+                len(indices) - len(deduped),
+            )
+        if self.settings.order == "selection":
+            return deduped
+        in_axis_order = np.sort(deduped)
+        if not np.array_equal(in_axis_order, deduped):
+            ez.logger.info(
+                "Slicer: selection %r tokens are not in axis order on axis %r; output "
+                "follows axis order (order='axis'). Set order='selection' to keep token order.",
+                self.settings.selection,
+                axis,
+            )
+        return in_axis_order
+
     def _reset_state(self, message: AxisArray) -> None:
         if self.settings.on_empty not in ("raise", "warn"):
             raise ValueError(f"on_empty must be 'raise' or 'warn', got {self.settings.on_empty!r}")
+        if self.settings.order not in ("axis", "selection"):
+            raise ValueError(f"order must be 'axis' or 'selection', got {self.settings.order!r}")
         axis = self.settings.axis or message.dims[-1]
         axis_idx = message.get_axis_idx(axis)
         axinfo = message.axes.get(axis, None)
@@ -237,7 +287,7 @@ def _reset_state(self, message: AxisArray) -> None:
             # Empty _slices (nothing matched) -> select no entries (0-length),
             # rather than np.hstack([]) which would raise.
             indices = np.hstack([indices[_] for _ in _slices]) if _slices else indices[:0]
-            self._state.slice_ = np.s_[indices]
+            self._state.slice_ = np.s_[self._normalize_indices(indices, axis)]
 
         # Create the output axis
         if axis in message.axes and hasattr(message.axes[axis], "data") and len(message.axes[axis].data) > 0:
@@ -274,6 +324,7 @@ def slicer(
     selection: str = "",
     axis: str | None = None,
     field: str | None = None,
+    order: str = "axis",
     on_empty: str = "warn",
 ) -> SlicerTransformer:
     """
@@ -284,10 +335,14 @@ def slicer(
         axis: The name of the axis to slice along. If None, the last axis is used.
         field: Which field of a structured coordinate axis to match selection values
           against. See :obj:`SlicerSettings` for details.
+        order: "axis" (default) or "selection" — how to order the entries a
+          comma-separated selection resolves to. See :obj:`SlicerSettings` for details.
         on_empty: "warn" (default) or "raise" — what to do when a label/regex
           selection matches nothing. See :obj:`SlicerSettings` for details.
 
     Returns:
         :obj:`SlicerTransformer`
     """
-    return SlicerTransformer(SlicerSettings(selection=selection, axis=axis, field=field, on_empty=on_empty))
+    return SlicerTransformer(
+        SlicerSettings(selection=selection, axis=axis, field=field, order=order, on_empty=on_empty)
+    )

tests/unit/test_slicer.py

@@ -362,3 +362,88 @@ def test_slicer_single_label_match_keeps_axis():
 def test_slicer_on_empty_invalid():
     with pytest.raises(ValueError, match="on_empty"):
         SlicerTransformer(SlicerSettings(selection="C.*", axis="ch", on_empty="ignore"))(_make_on_empty_msg())
+
+
+def _make_order_msg() -> AxisArray:
+    n_chans = 12
+    return AxisArray(
+        data=np.arange(2 * n_chans, dtype=float).reshape(2, n_chans),
+        dims=["time", "ch"],
+        axes={
+            "time": AxisArray.TimeAxis(fs=10.0),
+            "ch": AxisArray.CoordinateAxis(data=np.array([f"ch{i:02d}" for i in range(n_chans)]), dims=["ch"]),
+        },
+        key="order",
+    )
+
+
+def test_slicer_order_axis_default(caplog):
+    """Default order="axis": a selection is a filter — overlapping tokens are
+    deduplicated and the output follows axis order regardless of token order,
+    with logs signaling that normalization changed something (issue #188)."""
+    msg = _make_order_msg()
+    with caplog.at_level(logging.INFO, logger="ezmsg"):
+        out = SlicerTransformer(SlicerSettings(selection="3:10,1,7", axis="ch"))(msg)
+    expected = [1, 3, 4, 5, 6, 7, 8, 9]
+    assert np.array_equal(out.data, msg.data[:, expected])
+    assert [str(x) for x in out.axes["ch"].data] == [f"ch{i:02d}" for i in expected]
+    assert any("removed 1 duplicate" in rec.getMessage() for rec in caplog.records)
+    assert any("not in axis order" in rec.getMessage() for rec in caplog.records)
+
+    # An already-normalized selection is untouched and logs nothing.
+    caplog.clear()
+    with caplog.at_level(logging.INFO, logger="ezmsg"):
+        out = SlicerTransformer(SlicerSettings(selection="1,3:10", axis="ch"))(msg)
+    assert np.array_equal(out.data, msg.data[:, expected])
+    assert not caplog.records
+
+
+def test_slicer_order_axis_label_tokens():
+    """Regex/label selections naming the same channels in a different token order
+    produce identical output (issue #188)."""
+    labels = np.array(["elec1-m1-1", "elec1-m1-2", "elec2-aip-1", "elec2-aip-2"])
+    msg = AxisArray(
+        data=np.arange(2 * 4, dtype=float).reshape(2, 4),
+        dims=["time", "ch"],
+        axes={
+            "time": AxisArray.TimeAxis(fs=10.0),
+            "ch": AxisArray.CoordinateAxis(data=labels, dims=["ch"]),
+        },
+        key="order_labels",
+    )
+    out_fwd = SlicerTransformer(SlicerSettings(selection=".*-m1-.*,.*-aip-.*", axis="ch"))(msg)
+    out_rev = SlicerTransformer(SlicerSettings(selection=".*-aip-.*,.*-m1-.*", axis="ch"))(msg)
+    assert [str(x) for x in out_fwd.axes["ch"].data] == list(labels)
+    assert [str(x) for x in out_rev.axes["ch"].data] == list(labels)
+    assert np.array_equal(out_fwd.data, out_rev.data)
+    # Overlapping positional/label tokens do not duplicate a channel.
+    out_dup = SlicerTransformer(SlicerSettings(selection="0:3,1", axis="ch"))(msg)
+    assert [str(x) for x in out_dup.axes["ch"].data] == list(labels[:3])
+
+
+def test_slicer_order_selection():
+    """order="selection" keeps token order (intentional permutation) but still
+    removes duplicates, keeping the first occurrence."""
+    msg = _make_order_msg()
+    out = SlicerTransformer(SlicerSettings(selection="3,1,2", axis="ch", order="selection"))(msg)
+    assert np.array_equal(out.data, msg.data[:, [3, 1, 2]])
+    assert [str(x) for x in out.axes["ch"].data] == ["ch03", "ch01", "ch02"]
+
+    out = SlicerTransformer(SlicerSettings(selection="3:10,1,7", axis="ch", order="selection"))(msg)
+    expected = [3, 4, 5, 6, 7, 8, 9, 1]
+    assert np.array_equal(out.data, msg.data[:, expected])
+    assert [str(x) for x in out.axes["ch"].data] == [f"ch{i:02d}" for i in expected]
+
+
+def test_slicer_order_single_slice_untouched():
+    """A single non-comma slice token bypasses normalization: an explicit
+    reverse slice remains a reverse."""
+    msg = _make_order_msg()
+    out = SlicerTransformer(SlicerSettings(selection="::-1", axis="ch"))(msg)
+    assert np.array_equal(out.data, msg.data[:, ::-1])
+    assert [str(x) for x in out.axes["ch"].data] == [f"ch{i:02d}" for i in range(11, -1, -1)]
+
+
+def test_slicer_order_invalid():
+    with pytest.raises(ValueError, match="order"):
+        SlicerTransformer(SlicerSettings(selection="0:2", axis="ch", order="token"))(_make_order_msg())