FIX: browser defects found by a corner-case sweep

Route variable-duration browsing on a backend capability flag rather than
the backend name, so mne-qt-browser can opt in. Duck-typed the way
BrowserBase._has_time_slice already is, so an older mne-qt-browser still
declines. Six tests whose skip reason claimed matplotlib-only now run on
both backends.

Three of these fixes are regressions on the *equal-duration* path, not
ragged-only:

- _recompute_epochs_vlines computed an unclamped sample offset, so a click
  in an epoch's last half sample rounded one sample past its end, a latency
  no epoch holds. Every line was dropped while the readout still showed the
  out-of-range value. The Qt backend kept its old path behind a guard; the
  matplotlib rewrite had none, so fixed-duration data went through the new
  code.
- _draw_traces rebuilt the visible-epoch list by searchsorting the time
  range, which drops the last epoch when it holds one sample, and raises
  when that is the only visible epoch. Ask the view instead.
- The colour band mask excluded each epoch's own first sample, so a
  one-sample epoch was drawn in its neighbour's colour and the window's
  first sample was never painted at all.

Ragged-only:

- _create_epoch_histogram called np.ptp(..., axis=2) on a list of arrays.
  Peak-to-peak is per trial, so compute it per epoch.
- _getitem moved the per-epoch bounds but never re-derived the union time
  axis, so as_fixed() kept padding out to epochs that had been dropped:
  epochs[0] of a 100-sample epoch returned (1, 3, 280) with 540 NaN and
  n_contributing == 0 at 180 time points. crop() already re-derives it.
- drop_bad(reject=...) reached Epochs.times and raised an internal error
  with no classification; it now declines clearly. The no-arg call still
  short-circuits, which _concatenate_epochs relies on.

Also silence two ty diagnostics in _crop_variable that predate this branch.

Verified against the pre-PR commit across 7,697 recorded states and 377
figures per environment: all 32 fields that determine what the reader sees
are bit-identical, and the only movement is the vline landing on a real
sample instead of between two.

Author: snesmaeili

mne/epochs.py

@@ -1978,6 +1978,19 @@ def drop_bad(
             flat = self.flat
         if any(isinstance(rej, str) and rej != "existing" for rej in (reject, flat)):
             raise ValueError('reject and flat, if strings, must be "existing"')
+        if self._variable_duration and (reject or flat):
+            # the no-arg call has already short-circuited above, so reaching
+            # here means real thresholds; amplitude rejection would run per
+            # epoch, but it goes through `times` on the way and there is no
+            # ragged path for it yet
+            raise NotImplementedError(
+                "drop_bad() with reject or flat is not implemented for "
+                "variable-duration epochs. Amplitude rejection is per-trial "
+                "and could work here, but the preloaded path needs one shared "
+                "time axis, which these epochs do not have. Pass reject= to "
+                "Epochs() at construction instead, which does apply it per "
+                "epoch. See https://github.com/mne-tools/mne-python/issues/14206."
+            )
         self._reject_setup(reject, flat, allow_callable=True)
         self._get_data(out=False, verbose=verbose)
         return self
@@ -2855,8 +2868,8 @@ def _crop_variable(self, tmin, tmax, include_tmax):
             self._tmax_per_epoch = None  # ty: ignore[invalid-assignment]
             start_idx, stop_idx = first_idx[0], first_idx[0] + lengths[0] - 1
         else:
-            start_idx = int(round(starts.min() * sfreq))
-            stop_idx = int(round(stops.max() * sfreq))
+            start_idx = int(round(float(np.min(starts)) * sfreq))
+            stop_idx = int(round(float(np.max(stops)) * sfreq))
         self._raw_times = np.arange(start_idx, stop_idx + 1) / sfreq
         self._set_times(self._raw_times)
 

mne/utils/mixin.py

@@ -277,6 +277,15 @@ def _getitem(
             # an ndarray takes a slice or an index array equally well
             inst._tmin_per_epoch = inst._tmin_per_epoch[select]
             inst._tmax_per_epoch = inst._tmax_per_epoch[select]
+            # the union window is defined by those bounds, so it has to be
+            # re-derived here as `crop` does; otherwise `as_fixed` keeps
+            # padding out to epochs that are no longer present
+            if len(inst._tmin_per_epoch):
+                sfreq = float(inst.info["sfreq"])
+                start_idx = int(round(inst._tmin_per_epoch.min() * sfreq))
+                stop_idx = int(round(inst._tmax_per_epoch.max() * sfreq))
+                inst._raw_times = np.arange(start_idx, stop_idx + 1) / sfreq
+                inst._set_times(inst._raw_times)
         if drop_event_id:
             # update event id to reflect new content of inst
             inst.event_id = {

mne/viz/_figure.py

@@ -710,7 +710,11 @@ def _create_epoch_histogram(self):
         """Create peak-to-peak histogram of channel amplitudes."""
         epochs = self.mne.inst
         data = OrderedDict()
-        ptp = np.ptp(epochs.get_data(copy=False), axis=2)
+        # per epoch, so that variable-duration epochs (a list of arrays)
+        # work too; peak-to-peak is a per-trial reduction either way
+        ptp = np.array(
+            [np.ptp(epoch, axis=-1) for epoch in epochs.get_data(copy=False)]
+        )
         for ch_type in ("eeg", "mag", "grad"):
             if ch_type in epochs:
                 data[ch_type] = ptp.T[self.mne.ch_types == ch_type].ravel()
@@ -808,6 +812,27 @@ def _load_backend(backend_name):
     return backend
 
 
+def _check_variable_duration_backend():
+    """Raise unless the active browser backend can draw ragged epochs.
+
+    Matplotlib always can. The Qt backend gained the ability in
+    mne-qt-browser 0.8, which announces it with a module-level flag, so an
+    older one declines here rather than drawing a wrong picture from a
+    boundary model it does not know about.
+    """
+    backend_name = get_browser_backend()
+    if backend_name == "matplotlib":
+        return
+    module = _load_backend(backend_name)
+    if not getattr(module, "_SUPPORTS_VARIABLE_DURATION", False):
+        raise NotImplementedError(
+            f"Browsing variable-duration epochs is not implemented for the "
+            f"{backend_name} backend of this version, only for matplotlib. "
+            "Upgrade mne-qt-browser, or select matplotlib with "
+            'mne.viz.set_browser_backend("matplotlib").'
+        )
+
+
 def _get_browser(show, block, **kwargs):
     """Instantiate a new MNE browse-style figure."""
     from .utils import _get_figsize_from_config

mne/viz/_mpl_figure.py

@@ -2263,8 +2263,10 @@ def _draw_traces(self):
         # check for bad epochs
         time_range = (self.mne.times + self.mne.first_time)[[0, -1]]
         if self.mne.instance_type == "epochs":
-            epoch_ix = np.searchsorted(self.mne.boundary_times, time_range)
-            epoch_ix = np.arange(epoch_ix[0], epoch_ix[1])
+            # ask the view directly: deriving this from the time range drops
+            # the last epoch whenever it holds a single sample, because that
+            # sample's time is its own left boundary
+            epoch_ix = np.arange(*self._get_epoch_ix_range())
             epoch_nums = self.mne.inst.selection[epoch_ix[0] : epoch_ix[-1] + 1]
             (visible_bad_epoch_ix,) = np.isin(epoch_nums, self.mne.bad_epochs).nonzero()
             while len(self.mne.epoch_traces):
@@ -2334,7 +2336,11 @@ def _draw_traces(self):
                     _starts = self.mne.boundary_times[epoch_ix][bool_ixs]
                     _stops = self.mne.boundary_times[epoch_ix + 1][bool_ixs]
                     for _start, _stop in zip(_starts, _stops):
-                        _mask = np.logical_and(_start < this_times, this_times <= _stop)
+                        # inclusive at both ends: an epoch owns its own first
+                        # sample, and a one-sample epoch has nothing else
+                        _mask = np.logical_and(
+                            _start <= this_times, this_times <= _stop
+                        )
                         mask = mask | _mask
                     _times = np.ma.masked_array(this_times, mask=~mask)
                     # always use the existing traces first
@@ -2442,7 +2448,15 @@ def _recompute_epochs_vlines(self, xdata):
                 len(boundary_times) - 2,
             )
         )
-        offset = round((xdata - boundary_times[clicked_ix]) * sfreq)
+        # clamp into the clicked epoch: a click in its last half sample would
+        # otherwise round up to one sample past its end, which no epoch holds,
+        # and every line would be dropped
+        n_samp = int(
+            round((boundary_times[clicked_ix + 1] - boundary_times[clicked_ix]) * sfreq)
+        )
+        offset = int(
+            np.clip(round((xdata - boundary_times[clicked_ix]) * sfreq), 0, n_samp - 1)
+        )
         latency = self.mne.epoch_tmins[clicked_ix] + offset / sfreq
         ix_start, ix_stop = self._get_epoch_ix_range()
         xs = list()

mne/viz/epochs.py

@@ -1133,15 +1133,9 @@ def plot_epochs(
     )
 
     if epochs._variable_duration:
-        from ._figure import get_browser_backend
-
-        backend_name = get_browser_backend()
-        if backend_name != "matplotlib":
-            raise NotImplementedError(
-                f"Browsing variable-duration epochs is not implemented for the "
-                f"{backend_name} backend yet, only for matplotlib. Select it "
-                'with mne.viz.set_browser_backend("matplotlib").'
-            )
+        from ._figure import _check_variable_duration_backend
+
+        _check_variable_duration_backend()
 
     fig = _get_browser(show=show, block=block, **params)
 

mne/viz/tests/test_epochs.py

@@ -554,8 +554,6 @@ def variable_epochs():
 
 def test_plot_variable_duration_is_native(variable_epochs, browser_backend):
     """Test that browsing ragged epochs neither warns nor pads."""
-    if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
 
     def _boom(*args, **kwargs):
         raise AssertionError("plot() fell back to as_fixed() instead of browsing")
@@ -569,8 +567,6 @@ def _boom(*args, **kwargs):
 
 def test_plot_variable_duration_boundaries(variable_epochs, browser_backend):
     """Test that the browser lays epochs end to end at their true lengths."""
-    if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
     fig = variable_epochs.plot(n_epochs=2)
     assert_allclose(fig.mne.boundary_times, _boundaries())
     assert_array_equal(fig.mne.boundary_samples, np.r_[0, np.cumsum(LENGTHS_VAR)])
@@ -583,8 +579,6 @@ def test_plot_variable_duration_window_spans_whole_epochs(
     variable_epochs, browser_backend
 ):
     """Test that n_epochs means epochs, not a representative duration."""
-    if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
     boundaries = _boundaries()
     for n_epochs in (1, 2, 3, 4):
         fig = variable_epochs.plot(n_epochs=n_epochs)
@@ -594,8 +588,6 @@ def test_plot_variable_duration_window_spans_whole_epochs(
 
 def test_plot_variable_duration_data_is_unpadded(variable_epochs, browser_backend):
     """Test that a view holds exactly the source samples, in order."""
-    if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
     fig = variable_epochs.plot(n_epochs=2)
     source = variable_epochs.get_data()
     for keys in ([], ["right"], ["right", "right"]):
@@ -616,8 +608,6 @@ def test_plot_variable_duration_data_is_unpadded(variable_epochs, browser_backen
 
 def test_plot_variable_duration_navigation(variable_epochs, browser_backend):
     """Test that arrow keys move by epochs and land on real boundaries."""
-    if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
     boundaries = _boundaries()
     fig = variable_epochs.plot(n_epochs=2)
     assert fig.mne.t_start == pytest.approx(boundaries[0])
@@ -642,8 +632,6 @@ def test_plot_variable_duration_navigation(variable_epochs, browser_backend):
 
 def test_plot_variable_duration_home_end(variable_epochs, browser_backend):
     """Test that home/end change the epoch count and recompute the duration."""
-    if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
     boundaries = _boundaries()
     fig = variable_epochs.plot(n_epochs=2)
     assert fig.mne.duration == pytest.approx(boundaries[2])
@@ -672,7 +660,7 @@ def test_plot_variable_duration_hscroll_patches(variable_epochs, browser_backend
 def test_plot_variable_duration_bad_epoch(variable_epochs, browser_backend):
     """Test that a click finds the right epoch when the widths differ."""
     if browser_backend.name != "matplotlib":
-        pytest.skip("variable-duration browsing is matplotlib-only")
+        pytest.skip("epoch marking by click is matplotlib-specific")
     boundaries = _boundaries()
     fig = variable_epochs.plot(n_epochs=4)
     y = fig.mne.traces[0].get_ydata()[0]
@@ -723,10 +711,14 @@ def test_plot_variable_duration_events(browser_backend):
     assert_allclose(got, want)
 
 
-def test_plot_variable_duration_refuses_other_backends(variable_epochs, monkeypatch):
-    """Test that non-matplotlib backends decline rather than fail obscurely."""
+def test_plot_variable_duration_refuses_old_backends(variable_epochs, monkeypatch):
+    """Test that a backend without the boundary model declines, not fails."""
     import mne.viz._figure
 
+    class _OldBackend:  # an mne-qt-browser that predates the boundary model
+        pass
+
     monkeypatch.setattr(mne.viz._figure, "get_browser_backend", lambda: "qt")
+    monkeypatch.setattr(mne.viz._figure, "_load_backend", lambda name: _OldBackend())
     with pytest.raises(NotImplementedError, match="not implemented for the qt"):
         variable_epochs.plot()