ENH: plot variable-duration epochs without padding

The epochs browser already draws its trials as a pseudo-continuous strip,
concatenating them and ruling a line at each boundary, so ragged epochs
need the samples they actually hold rather than a padded copy. plot()
leaves the not-implemented table and becomes native.

The x axis is built from those samples:

    lengths          = per-epoch sample counts
    boundary_samples = np.r_[0, np.cumsum(lengths)]
    boundary_times   = boundary_samples / sfreq
    n_times          = boundary_samples[-1]

_n_times_per_epoch returns len(times) when durations are equal, so this
is one code path and reproduces the previous uniform grid exactly while
never reading `times`, which variable-duration epochs refuse to provide.
A window of k epochs from index i spans boundary_times[i + k] minus
boundary_times[i]; _epoch_window computes that for both backends, and
_get_start_stop and _load_data share _get_epoch_ix_range so the sample
bounds and the concatenated array agree by construction. That is what
makes the existing shape assertions meaningful for ragged windows.

Arrow keys step whole epochs and shift steps whole windows, home and end
ask the boundaries how many seconds an epoch is worth, and the scrollbar
draws each epoch at its own width. Vertical lines mark a latency relative
to each epoch's own event and are omitted from epochs too short to reach
it, replacing arithmetic that took the remainder against one duration.
Events map through each epoch's own window; the fixed path keeps its
existing bounds, whose upper limit overshoots the last sample by |tmin|,
rather than have that copied.

_compute_scalings failed first of all, before any of the above, since it
reshaped _data as an array. ICA sources reach the same browser without
supplying the new per-epoch arrays, so those are derived from the
boundaries when absent.

Non-matplotlib backends decline with a message naming matplotlib until
mne-qt-browser can consume boundary_times, boundary_samples and n_times,
which the params dict now carries; the browser tests skip there for the
same reason.

Author: snesmaeili

mne/epochs.py

@@ -4066,7 +4066,6 @@ def _events_from_annotations(raw, events, event_id, annotations, on_missing):
 #: rather than a slow one, so they raise until implemented natively.
 _VARIABLE_NOT_IMPLEMENTED = {
     "filter": "filtering",
-    "plot": "browsing",
     "apply_function": "applying a function",
     "apply_baseline": "baseline correction",
     "crop": "cropping",

mne/tests/test_epochs_variable_duration.py

@@ -259,6 +259,13 @@ def test_display_methods_warn_and_fall_back(variable, meth):
         assert getattr(variable, meth)() is not None
 
 
+def test_plot_is_not_a_fallback():
+    """Test that browsing is native, not padded (see mne/viz/tests/test_epochs)."""
+    assert "plot" not in _VARIABLE_FALLBACK
+    assert "plot" not in _VARIABLE_NEEDS_POLICY
+    assert "plot" not in _VARIABLE_NOT_IMPLEMENTED
+
+
 # -- operations that stay native -------------------------------------------
 def test_pick_keeps_durations(variable):
     """Test that channel selection leaves the time axis alone."""

mne/viz/_figure.py

@@ -44,6 +44,37 @@ def __init__(self, **kwargs):
         vars(self).update(**kwargs)
 
 
+def _epoch_window(boundary_times, start_ix, n_epochs):
+    """Return the start time and duration of a window of whole epochs.
+
+    Epochs may differ in duration, so a window of ``n_epochs`` of them spans
+    whatever lies between the two boundaries rather than a fixed number of
+    seconds. ``start_ix`` is clamped so the requested epochs stay visible when
+    the object is long enough to allow it.
+
+    Parameters
+    ----------
+    boundary_times : array
+        Cumulative epoch edges in seconds, including both ends.
+    start_ix : int
+        Index of the first epoch to show.
+    n_epochs : int
+        Number of epochs to show.
+
+    Returns
+    -------
+    t_start : float
+        Time of the first boundary.
+    duration : float
+        Seconds spanned by the requested epochs.
+    """
+    n_total = len(boundary_times) - 1
+    n_epochs = int(np.clip(n_epochs, 1, n_total))
+    start_ix = int(np.clip(start_ix, 0, n_total - n_epochs))
+    stop_ix = start_ix + n_epochs
+    return boundary_times[start_ix], boundary_times[stop_ix] - boundary_times[start_ix]
+
+
 class BrowserBase(ABC):
     """A base class containing for the 2D browser.
 
@@ -78,7 +109,13 @@ def __init__(self, **kwargs):
                 f"Expected an instance of Raw, Epochs, or ICA, got {type(inst)}."
             )
 
-        if len(inst.times) < 2:
+        # variable-duration epochs have no one time axis, so count the samples
+        # the browser will lay end to end instead
+        if self.mne.instance_type == "epochs":
+            n_inst_times = self.mne.n_times
+        else:
+            n_inst_times = len(inst.times)
+        if n_inst_times < 2:
             raise ValueError(
                 "Data from at least two time points are required to open the browser."
             )
@@ -120,7 +157,12 @@ def __init__(self, **kwargs):
         self.mne.epoch_traces = list()
         self.mne.bad_epochs = list()
         if inst is not None:
-            self.mne.sampling_period = np.diff(inst.times[:2])[0] / inst.info["sfreq"]
+            # NB: this is 1 / sfreq**2, not a sampling period; it is only ever
+            # used as a small nudge before searchsorted on boundary_times, so
+            # keep the value while deriving it without touching inst.times
+            # (which variable-duration epochs refuse to provide).
+            sfreq = inst.info["sfreq"]
+            self.mne.sampling_period = (1.0 / sfreq) / sfreq
         # annotations
         self.mne.annotations = list()
         self.mne.hscroll_annotations = list()
@@ -157,6 +199,23 @@ def __init__(self, **kwargs):
             self.mne.midpoints = (
                 np.convolve(self.mne.boundary_times, np.ones(2), mode="valid") / 2
             )
+            # callers that only ever deal with equal-length epochs (ICA sources)
+            # do not supply these, so derive them from the boundaries
+            sfreq = self.mne.info["sfreq"]
+            if not hasattr(self.mne, "boundary_samples"):
+                self.mne.boundary_samples = np.round(
+                    np.asarray(self.mne.boundary_times) * sfreq
+                ).astype(int)
+            if not hasattr(self.mne, "epoch_tmins"):
+                n_epochs_total = len(self.mne.boundary_times) - 1
+                self.mne.epoch_tmins = np.full(
+                    n_epochs_total, float(self.mne.inst.times[0])
+                )
+                self.mne.epoch_tmaxs = (
+                    self.mne.epoch_tmins
+                    + np.diff(self.mne.boundary_times)
+                    - 1.0 / sfreq
+                )
 
         # initialize picks and projectors
         self._update_picks()
@@ -328,14 +387,36 @@ def _make_butterfly_selections_dict(self):
     # MANAGE DATA
     # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
 
+    def _get_epoch_ix_range(self):
+        """Return the first and last+1 epoch index currently in view.
+
+        Both :meth:`_get_start_stop` and :meth:`_load_data` go through here so
+        the sample bounds and the concatenated data cannot disagree, which is
+        what keeps the shape assertions in :meth:`_update_data` meaningful when
+        epochs differ in duration.
+        """
+        # subtract one sample from tstart before searchsorted, to make sure
+        # we land on the left side of the boundary time (avoid precision
+        # errors)
+        ix_start = int(
+            np.searchsorted(
+                self.mne.boundary_times, self.mne.t_start - self.mne.sampling_period
+            )
+        )
+        n_total = len(self.mne.boundary_times) - 1
+        ix_start = min(ix_start, max(n_total - 1, 0))
+        ix_stop = min(ix_start + self.mne.n_epochs, n_total)
+        return ix_start, ix_stop
+
     def _get_start_stop(self):
         # update time
         start_sec = self.mne.t_start - self.mne.first_time
         if self.mne.is_epochs:
-            start, stop = np.round(
-                np.array([start_sec, start_sec + self.mne.duration])
-                * self.mne.info["sfreq"]
-            ).astype(int)
+            # take the samples the visible epochs really hold, so that this
+            # agrees with _load_data by construction rather than by arithmetic
+            ix_start, ix_stop = self._get_epoch_ix_range()
+            start = int(self.mne.boundary_samples[ix_start])
+            stop = int(self.mne.boundary_samples[ix_stop])
         else:
             # ensure our end time includes the last sample
             disp_duration = (
@@ -355,13 +436,7 @@ def _load_data(self, start=None, stop=None):
             else:
                 return self.mne.inst[:, start:stop]
         else:
-            # subtract one sample from tstart before searchsorted, to make sure
-            # we land on the left side of the boundary time (avoid precision
-            # errors)
-            ix_start = np.searchsorted(
-                self.mne.boundary_times, self.mne.t_start - self.mne.sampling_period
-            )
-            ix_stop = ix_start + self.mne.n_epochs
+            ix_start, ix_stop = self._get_epoch_ix_range()
             item = slice(ix_start, ix_stop)
             data = np.concatenate(
                 self.mne.inst.get_data(item=item, copy=False), axis=-1

mne/viz/_mpl_figure.py

@@ -58,7 +58,7 @@
 from ..defaults import DEFAULTS
 from ..fixes import _close_event
 from ..utils import Bunch, _click_ch_name, logger
-from ._figure import BrowserBase
+from ._figure import BrowserBase, _epoch_window
 from .utils import (
     _BLIT_KWARGS,
     DraggableLine,
@@ -489,7 +489,7 @@ def __init__(self, inst, figsize, ica=None, xlabel="Time (s)", **kwargs):
             epoch_nums = self.mne.inst.selection
             for ix, _ in enumerate(epoch_nums):
                 start = self.mne.boundary_times[ix]
-                width = np.diff(self.mne.boundary_times[:2])[0]
+                width = self.mne.boundary_times[ix + 1] - start
                 ax_hscroll.add_patch(
                     Rectangle(
                         (start, 0),
@@ -791,12 +791,20 @@ def _keypress(self, event):
             old_t_start = self.mne.t_start
             direction = 1 if key.endswith("right") else -1
             if self.mne.is_epochs:
-                denom = 1 if key.startswith("shift") else self.mne.n_epochs
+                # step whole epochs, since they need not share a duration: one
+                # epoch normally, a whole window with shift
+                step = self.mne.n_epochs if key.startswith("shift") else 1
+                ix_start, _ = self._get_epoch_ix_range()
+                self.mne.t_start, self.mne.duration = _epoch_window(
+                    self.mne.boundary_times,
+                    ix_start + direction * step,
+                    self.mne.n_epochs,
+                )
             else:
                 denom = 1 if key.startswith("shift") else 4
-            t_max = last_time - self.mne.duration
-            t_start = self.mne.t_start + direction * self.mne.duration / denom
-            self.mne.t_start = np.clip(t_start, self.mne.first_time, t_max)
+                t_max = last_time - self.mne.duration
+                t_start = self.mne.t_start + direction * self.mne.duration / denom
+                self.mne.t_start = np.clip(t_start, self.mne.first_time, t_max)
             if self.mne.t_start != old_t_start:
                 self._update_hscroll()
                 self._redraw(annotations=True, skip_hscroll=True)
@@ -828,13 +836,17 @@ def _keypress(self, event):
             old_dur = self.mne.duration
             dur_delta = 1 if key == "end" else -1
             if self.mne.is_epochs:
+                ix_start, _ = self._get_epoch_ix_range()
                 # prevent from showing zero epochs, or more epochs than we have
-                self.mne.n_epochs = np.clip(
-                    self.mne.n_epochs + dur_delta, 1, len(self.mne.inst)
+                self.mne.n_epochs = int(
+                    np.clip(self.mne.n_epochs + dur_delta, 1, len(self.mne.inst))
+                )
+                # the epochs added or removed have their own durations, so ask
+                # the boundaries how many seconds that actually is
+                self.mne.t_start, new_dur = _epoch_window(
+                    self.mne.boundary_times, ix_start, self.mne.n_epochs
                 )
-                # use the length of one epoch as duration change
-                min_dur = len(self.mne.inst.times) / self.mne.info["sfreq"]
-                new_dur = self.mne.duration + dur_delta * min_dur
+                min_dur = np.diff(self.mne.boundary_times).min()
             else:
                 # never show fewer than 3 samples
                 min_dur = 3 * np.diff(self.mne.inst.times[:2])[0]
@@ -843,8 +855,10 @@ def _keypress(self, event):
                 new_dur = self.mne.duration * dur_delta
             self.mne.duration = np.clip(new_dur, min_dur, last_time)
             if self.mne.duration != old_dur:
-                if self.mne.t_start + self.mne.duration > last_time:
-                    self.mne.t_start = last_time - self.mne.duration
+                if not self.mne.is_epochs:
+                    if self.mne.t_start + self.mne.duration > last_time:
+                        self.mne.t_start = last_time - self.mne.duration
+                # (the epochs branch above already clamped t_start to a boundary)
                 self._update_hscroll()
                 self._redraw(annotations=True, skip_hscroll=True)
         elif key == "?":  # help window
@@ -998,7 +1012,10 @@ def _mouse_move(self, event):
             time = np.clip(time, self.mne.first_time, max_time)
             if self.mne.is_epochs:
                 ix = np.searchsorted(self.mne.boundary_times[1:], time, side="right")
-                time = self.mne.boundary_times[ix]
+                # the epochs from here on have their own durations
+                time, self.mne.duration = _epoch_window(
+                    self.mne.boundary_times, ix, self.mne.n_epochs
+                )
             if self.mne.t_start != time:
                 self.mne.t_start = time
                 self._update_hscroll()
@@ -1917,7 +1934,10 @@ def _check_update_hscroll_clicked(self, event):
         time = np.clip(time, self.mne.first_time, max_time)
         if self.mne.is_epochs:
             ix = np.searchsorted(self.mne.boundary_times[1:], time, side="right")
-            time = self.mne.boundary_times[ix]
+            # the epochs from here on have their own durations
+            time, self.mne.duration = _epoch_window(
+                self.mne.boundary_times, ix, self.mne.n_epochs
+            )
         if self.mne.t_start != time:
             self.mne.t_start = time
             self._update_hscroll()
@@ -2252,11 +2272,9 @@ def _draw_traces(self):
             # handle custom epoch colors (for autoreject integration)
             if self.mne.epoch_colors is None:
                 # shape: n_traces × RGBA → n_traces × n_epochs × RGBA
-                custom_colors = np.tile(
-                    ch_colors[:, None, :], (1, self.mne.n_epochs, 1)
-                )
+                custom_colors = np.tile(ch_colors[:, None, :], (1, len(epoch_ix), 1))
             else:
-                custom_colors = np.empty((len(self.mne.picks), self.mne.n_epochs, 4))
+                custom_colors = np.empty((len(self.mne.picks), len(epoch_ix), 4))
                 for ii, _epoch_ix in enumerate(epoch_ix):
                     this_colors = self.mne.epoch_colors[_epoch_ix]
                     custom_colors[:, ii] = to_rgba_array(
@@ -2408,26 +2426,43 @@ def _recompute_epochs_vlines(self, xdata):
         # special case: changed view duration w/ "home" or "end" key
         # (no click event, hence no xdata)
         if xdata is None:
-            xdata = np.array(self.mne.vline.get_segments())[0, 0, 0]
-        # compute the (continuous) times for the lines on each epoch
-        epoch_dur = np.diff(self.mne.boundary_times[:2])[0]
-        rel_time = xdata % epoch_dur
-        abs_time = self.mne.times[0]
-        xs = np.arange(self.mne.n_epochs) * epoch_dur + abs_time + rel_time
-        segs = np.array(self.mne.vline.get_segments())
+            segments = self.mne.vline.get_segments()
+            if not len(segments):  # no visible epoch reaches that latency
+                return None
+            xdata = np.array(segments)[0, 0, 0]
+        # Work out which latency relative to its own event was clicked, then
+        # mark that same latency on every visible epoch. Epochs need not share a
+        # duration, so an epoch that never reaches this latency gets no line.
+        sfreq = self.mne.info["sfreq"]
+        boundary_times = self.mne.boundary_times
+        clicked_ix = int(
+            np.clip(
+                np.searchsorted(boundary_times[1:], xdata, side="right"),
+                0,
+                len(boundary_times) - 2,
+            )
+        )
+        offset = round((xdata - boundary_times[clicked_ix]) * sfreq)
+        latency = self.mne.epoch_tmins[clicked_ix] + offset / sfreq
+        ix_start, ix_stop = self._get_epoch_ix_range()
+        xs = list()
+        for ix in range(ix_start, ix_stop):
+            tmin, tmax = self.mne.epoch_tmins[ix], self.mne.epoch_tmaxs[ix]
+            if tmin - 0.5 / sfreq <= latency <= tmax + 0.5 / sfreq:
+                xs.append(boundary_times[ix] + (latency - tmin))
+        xs = np.array(xs, float)
         # recreate segs from scratch in case view duration changed
         # (i.e., handle case when n_segments != n_epochs)
         segs = np.tile([[0.0], [1.0]], (len(xs), 1, 2))  # y values
-        segs[..., 0] = np.tile(xs[:, None], 2)  # x values
+        segs[..., 0] = np.tile(xs[:, None], 2) if len(xs) else segs[..., 0]
         self.mne.vline.set_segments(segs)
-        return rel_time
+        return latency
 
     def _show_vline(self, xdata):
         """Show the vertical line(s)."""
         if self.mne.is_epochs:
-            # convert xdata to be epoch-relative (for the text)
-            rel_time = self._recompute_epochs_vlines(xdata)
-            xdata = rel_time + self.mne.inst.times[0]
+            # the label shows the latency relative to each epoch's own event
+            xdata = self._recompute_epochs_vlines(xdata)
         else:
             self.mne.vline.set_xdata([xdata])
             self.mne.vline_hscroll.set_xdata([xdata])