Skip to content

Slicer: add on_empty setting — no-match selections warn and emit empty by default - #177

Merged
cboulay merged 3 commits into
devfrom
slicer-allow-empty
Jul 21, 2026
Merged

Slicer: add on_empty setting — no-match selections warn and emit empty by default#177
cboulay merged 3 commits into
devfrom
slicer-allow-empty

Conversation

@kylmcgr

@kylmcgr kylmcgr commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds an opt-in allow_empty setting to Slicer. By default a label/regex selection that matches nothing still raises; with allow_empty=True, non-matching tokens are dropped and a selection that matches nothing yields an empty (0-length) result plus a one-time warning.

Motivation

Today a label/regex selection that matches no labels raises ValueError("… matched no labels …"). That fail-fast is valuable — it catches typos, wrong-axis, and wrong-label mistakes — and stays the default.

But some callers apply the same selection across multiple streams where a given stream may legitimately contain none of the selected entries. The concrete case: a per-source region selection (e.g. ".*-aip-.*,.*-smg-.*") broadcast to every acquisition hub, where one hub carries none of those regions. There, "matched nothing" is expected, not an error — that source should simply contribute no channels. For those callers, raising is wrong; they want an empty result to flow through.

Changes

  • Add SlicerSettings.allow_empty: bool = False.
  • parse_slice(..., allow_empty=False): when a label/regex token matches nothing, return no indices instead of raising. In a comma-separated selection, non-matching tokens are dropped and matching ones kept (in order); if every token matches nothing the result is an empty tuple.
  • SlicerTransformer._reset_state: pass allow_empty through, guard the empty-result case (0-length selection along the axis, avoiding np.hstack([])), and log a warning once per stream configuration when the selection resolves to empty.
  • Thread the flag through the slicer() factory.

Behavior change

None by default — allow_empty=False preserves the existing raise exactly. The new behavior is strictly opt-in. Matching selections are unaffected regardless of the flag.

Testing

  • test_parse_slice_allow_empty: non-matching token → () (vs raise by default); comma list drops non-matching and keeps matching in order; all-non-matching → (); matching selection unaffected.
  • test_slicer_allow_empty_emits_empty: default raises; allow_empty=True on a no-match yields a (time, 0) output with a 0-length ch axis (time axis intact); a partial match keeps only the matching channels.
  • Full tests/unit/test_slicer.py passes (17 passed), including all pre-existing tests — no regressions.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an opt-in allow_empty flag to the Slicer stack so that label/regex selections that match nothing can yield an empty (0-length) result instead of raising, while preserving the existing fail-fast behavior by default.

Changes:

  • Extend parse_slice(...) with allow_empty=False to optionally treat no-match tokens as “no indices” rather than raising.
  • Thread allow_empty through SlicerSettings, SlicerTransformer._reset_state, and the slicer() factory; guard the empty-selection case to avoid np.hstack([]) and emit a warning.
  • Add unit tests covering both parse_slice(..., allow_empty=True) behavior and end-to-end empty outputs from SlicerTransformer.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ezmsg/sigproc/slicer.py Adds allow_empty plumbed through parsing, settings, transformer state reset, and the slicer() factory; handles empty selections safely and warns.
tests/unit/test_slicer.py Adds tests verifying allow-empty parsing behavior and that slicer emits 0-length channel outputs instead of raising when configured.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ezmsg/sigproc/slicer.py Outdated
Comment on lines +94 to +97
allow_empty: (Optional) If True, a label/regex token that matches nothing
returns no indices instead of raising. In a comma-separated selection,
non-matching tokens are dropped and the matching ones kept; if every
token matches nothing the result is an empty tuple (a 0-length slice).
cboulay added 2 commits July 21, 2026 18:53
… warn mode

- SlicerSettings.on_empty: 'raise' (default, unchanged behavior) or 'warn'.
- In warn mode, single-entry matches keep a length-1 axis instead of dropping
  the dimension (or crashing on exact-label matches via np.int64), so output
  rank is stable regardless of how many entries match.
- Log dropped non-matching tokens at info level in warn mode.
- parse_slice: cast exact-label hits to Python ints (np.int64 failed the
  isinstance(_, int) dim-drop check, producing 0-d axis data).
- on_empty now defaults to 'warn': a no-match selection emits an empty
  (0-length) result with a warning instead of raising; 'raise' remains
  available as the strict opt-in.
- Rank semantics no longer depend on on_empty: 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,
  preserving the documented parse_slice behavior in both modes.
@cboulay cboulay changed the title Slicer: add allow_empty option to emit empty instead of raising on no-match) Slicer: add on_empty setting — no-match selections warn and emit empty by default Jul 21, 2026
@cboulay

cboulay commented Jul 21, 2026

Copy link
Copy Markdown
Member

Thanks @kylmcgr — the motivation and the empty-result mechanics here were solid, and the original opt-in behavior is preserved (as on_empty="raise"). While reviewing I found some issues in the surrounding single-match paths and, after discussion, decided to change the API shape and the default. I pushed two commits on top of yours; summary of what changed and why:

4699a68allow_empty: boolon_empty: "raise" | "warn", and the one-match case

Exercising the branch with selections that resolve to exactly one entry turned up two problems (both pre-existing on dev, but this PR's broadcast-a-selection-to-many-streams use case makes them likely to be hit):

  • An exact single-label match (e.g. "XYZ.*, O1" where only O1 exists) crashed with ValueError: dims must be same length as data.shapenp.where returns np.int64, which fails the isinstance(_, int) dim-drop check in _reset_state, producing 0-d axis data.
  • A regex single match (e.g. "XYZ.*, O.*" matching one channel) silently dropped the sliced axis, so output rank depended on how many entries matched: 0 matches → (t, 0), 1 match → (t,), 2 matches → (t, 2).

So: allow_empty became on_empty: "raise" | "warn" (an invalid value raises), single-entry matches now keep a length-1 axis, exact-label hits are cast to Python int in parse_slice, and warn mode logs dropped tokens at info level so typos in a comma list remain discoverable.

8dc1b67 — default flipped to "warn", rank semantics decoupled from mode

We decided the default should be lenient: the "matched nothing" error fires at stream time in _reset_state, killing a running pipeline unit, which is a harsh failure mode when one source legitimately carries none of the selected entries. on_empty="raise" remains the strict opt-in.

Flipping the default surfaced a conflict: warn mode's "always keep the axis" rule would have broken the documented dim-drop for bare positional selections like "5". Resolution — rank behavior no longer depends on on_empty at all:

  • Bare-integer positional selections ("5") drop the dimension, as documented, in both modes.
  • Label/regex selections always preserve the sliced axis — a single matching entry yields a length-1 axis. (Exact single-label matches crashed before, so only the single-hit-regex case is a real behavior change.)

Net behavior change vs. dev

  • A label/regex selection that matches nothing now warns and emits a 0-length result by default (set on_empty="raise" for the old fail-fast).
  • A label/regex selection resolving to a single entry keeps a length-1 axis instead of dropping the dimension (regex case) or crashing (exact case).
  • Positional/slice selections are unchanged.

All slicer tests (19) and the full unit suite (3595 passed) pass against the branch.

@cboulay
cboulay merged commit 1d9ebe2 into dev Jul 21, 2026
14 checks passed
@cboulay
cboulay deleted the slicer-allow-empty branch July 21, 2026 23:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants