From 745a92630eced6c0e056541a01a4347d1a5daf12 Mon Sep 17 00:00:00 2001 From: payam Date: Tue, 25 Aug 2026 14:52:17 +0200 Subject: [PATCH 1/2] change peak if overlay changes --- mne/viz/_brain/_brain.py | 42 ++++++++++++++++++++++++++--- mne/viz/_brain/tests/test_brain.py | 43 ++++++++++++++++++++++++++++++ mne/viz/backends/_pyvista.py | 7 +++-- 3 files changed, 87 insertions(+), 5 deletions(-) diff --git a/mne/viz/_brain/_brain.py b/mne/viz/_brain/_brain.py index b670d767735..5a93938a09b 100644 --- a/mne/viz/_brain/_brain.py +++ b/mne/viz/_brain/_brain.py @@ -581,6 +581,7 @@ def setup_time_viewer(self, time_viewer=True, show_traces=True): self._picked_patches = {key: list() for key in all_keys} self._picked_points = dict() self._peak_vertices = {} + self._auto_peak_points = set() self._trace_meta = {} self._mouse_no_mvt = -1 self._show_hover_info = False @@ -920,6 +921,13 @@ def _configure_dock_colormap_widget(self, name): def select_data_key(value): self._active_data_key = value self._refresh_colormap_widgets() + self._update_act_data_smooth() + if self.show_traces: + self._update_peak_vertices() + if self.mpl_canvas is not None: + self.mpl_canvas.axes.relim() + self.mpl_canvas.axes.autoscale_view() + self.mpl_canvas.update_plot() self.widgets["data_key"] = self._renderer._dock_add_combo_box( name="Overlay", @@ -1205,6 +1213,12 @@ def _configure_vertex_time_course(self): self.plot_time_line(update=False) # then the picked points + self._update_peak_vertices() + + def _update_peak_vertices(self): + """(Re)compute the peak vertex per hemi for the active overlay.""" + old_peak_vertices = self._peak_vertices + self._peak_vertices = {} for idx, hemi in enumerate(["lh", "rh", "vol"]): act_data = self.act_data_smooth.get(hemi, [None])[0] if act_data is None: @@ -1228,16 +1242,27 @@ def _configure_vertex_time_course(self): ) vertex_id = vertices[ind[0]] self._peak_vertices[hemi] = vertex_id + + old_vertex_id = old_peak_vertices.get(hemi) + if old_vertex_id == vertex_id: + self._auto_peak_points.add((hemi, vertex_id)) + continue + was_auto_picked = (hemi, old_vertex_id) in self._auto_peak_points + if old_vertex_id is not None and was_auto_picked: + self._remove_vertex_glyph(hemi=hemi, vertex_id=old_vertex_id) + self._auto_peak_points.add((hemi, vertex_id)) publish( self, VertexSelect(hemi=hemi, vertex_id=vertex_id, source_id=ind[0]), ) + if self.mpl_canvas is not None: + self.mpl_canvas.sync_traces() - def _configure_picking(self): + def _update_act_data_smooth(self): # get data for each hemi from scipy.sparse import csr_array - for idx, hemi in enumerate(["vol", "lh", "rh"]): + for hemi in ["vol", "lh", "rh"]: hemi_data = self._data.get(hemi) if hemi_data is not None: act_data = hemi_data["array"] @@ -1252,6 +1277,9 @@ def _configure_picking(self): ) self.act_data_smooth[hemi] = (act_data, smooth_mat) + def _configure_picking(self): + self._update_act_data_smooth() + self._renderer._update_picking_callback( self._on_mouse_move, self._on_button_press, @@ -1656,6 +1684,7 @@ def _add_vertex_glyph(self, hemi, mesh, vertex_id, update=True): rindex = lst.index(self._picked_renderer) row, col = self._renderer._index_to_loc(rindex) + is_peak = self._peak_vertices.get(hemi) == vertex_id spheres = list() for _ in self._iter_views(hemi): # Using _sphere() instead of renderer.sphere() for 2 reasons: @@ -1667,8 +1696,14 @@ def _add_vertex_glyph(self, hemi, mesh, vertex_id, update=True): actor, mesh = self._renderer._sphere( center=np.array(center), color=color, - radius=4.0, + radius=4.5 if is_peak else 3.0, + resolution=24 if is_peak else 8, ) + if is_peak: + prop = actor.GetProperty() + prop.SetSpecular(0.6) + prop.SetSpecularPower(40) + prop.SetSpecularColor(1, 1, 1) spheres.append(dict(mesh=mesh, actor=actor)) # add metadata for picking @@ -1686,6 +1721,7 @@ def _remove_vertex_glyph(self, *, hemi, vertex_id, render=True): # to all linked brains, so by the time a given brain's own loop (e.g. # in clear_glyphs) reaches this (hemi, vertex_id) it may already be # gone; just no-op in that case. + self._auto_peak_points.discard((hemi, vertex_id)) spheres = self._picked_points.pop((hemi, vertex_id), None) if spheres is None: return diff --git a/mne/viz/_brain/tests/test_brain.py b/mne/viz/_brain/tests/test_brain.py index d319e043e03..0eb6ec4d811 100644 --- a/mne/viz/_brain/tests/test_brain.py +++ b/mne/viz/_brain/tests/test_brain.py @@ -1543,6 +1543,49 @@ def row_text(row): new_peak_line = next(iter(brain._picked_points.values()))[0]["line"] assert new_peak_line.get_color() == peak_color + # the auto-picked "Peak" trace must follow the active overlay + peak1 = brain._peak_vertices["lh"] + n_verts = len(brain.geo["lh"].coords) + peak2 = n_verts - 1 if peak1 != n_verts - 1 else n_verts - 2 + manual_vertex = next(v for v in (0, 1, 2) if v not in (peak1, peak2)) + ui_events.publish(brain, ui_events.VertexSelect(hemi="lh", vertex_id=manual_vertex)) + assert ("lh", manual_vertex) in brain._picked_points + time = brain._all_data["data"]["time"] + array2 = np.zeros((n_verts, len(time))) + array2[peak2] = 1.0 + brain.add_data( + array2, + fmin=0.0, + fmid=0.5, + fmax=1.0, + vertices=np.arange(n_verts), + time=time, + hemi="lh", + colormap="hot", + key="data2", + remove_existing=False, # keep "data" around so we can switch back + ) + assert brain._active_data_key == "data2" + assert brain._peak_vertices["lh"] == peak2 + assert ("lh", peak2) in brain._picked_points + assert ("lh", peak1) not in brain._picked_points + assert ("lh", manual_vertex) in brain._picked_points # manual pick untouched + + # switching back to the original overlay restores its peak + brain.widgets["data_key"].set_value("data") + assert brain._peak_vertices["lh"] == peak1 + assert ("lh", peak1) in brain._picked_points + assert ("lh", peak2) not in brain._picked_points + assert ("lh", manual_vertex) in brain._picked_points # still untouched + row_lines2 = [rows.itemAt(i).widget()._line for i in range(rows.count())] + peak_row2 = next( + rows.itemAt(i).widget() + for i, ln in enumerate(row_lines2) + if brain._trace_meta.get(ln, (None,))[0] == "lh" + and brain._trace_meta[ln][1] == peak1 + ) + assert row_text(peak_row2) == f"Peak (LH) {peak1}" + def _send_mouse_move(widget, point, buttons=None): """Deliver a synthetic Qt mouse move (QTest.mouseMove warps the real cursor).""" diff --git a/mne/viz/backends/_pyvista.py b/mne/viz/backends/_pyvista.py index 0b44afe816e..acbafe072a9 100644 --- a/mne/viz/backends/_pyvista.py +++ b/mne/viz/backends/_pyvista.py @@ -1070,9 +1070,12 @@ def _set_volume_range(self, volume, ctable, alpha, scalar_bar, rng, fmt=None): def _update_volume_rgba(self, grid, ctable, rng): _update_volume_rgba(grid, ctable, rng) - def _sphere(self, center, color, radius): + def _sphere(self, center, color, radius, *, resolution=8): mesh = pyvista.Sphere( - radius=radius, center=center, theta_resolution=8, phi_resolution=8 + radius=radius, + center=center, + theta_resolution=resolution, + phi_resolution=resolution, ) actor = _add_mesh(self.plotter, mesh=mesh, color=color) return actor, mesh From 3c6c9360e9573b7437a2004e8797a4aa48217292 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Tue, 25 Aug 2026 20:38:43 +0200 Subject: [PATCH 2/2] FIX: Tests, DRY --- doc/changes/dev/14222.newfeature.rst | 1 + mne/viz/_brain/_brain.py | 52 ++++++++++++++------ mne/viz/_brain/tests/test_brain.py | 73 +++++++++++++++++++++++++--- 3 files changed, 105 insertions(+), 21 deletions(-) create mode 100644 doc/changes/dev/14222.newfeature.rst diff --git a/doc/changes/dev/14222.newfeature.rst b/doc/changes/dev/14222.newfeature.rst new file mode 100644 index 00000000000..3d33d427a84 --- /dev/null +++ b/doc/changes/dev/14222.newfeature.rst @@ -0,0 +1 @@ +The auto-picked peak vertex and its activity trace in the :class:`mne.viz.Brain` GUI now follow the active overlay, by `Payam Sadeghi-Shabestari`_. diff --git a/mne/viz/_brain/_brain.py b/mne/viz/_brain/_brain.py index 5a93938a09b..f14a5fdd8ae 100644 --- a/mne/viz/_brain/_brain.py +++ b/mne/viz/_brain/_brain.py @@ -1217,6 +1217,11 @@ def _configure_vertex_time_course(self): def _update_peak_vertices(self): """(Re)compute the peak vertex per hemi for the active overlay.""" + if self.traces_mode != "vertex": + # in label mode a VertexSelect would toggle the label containing + # the peak (and label extraction may not even be possible, e.g., + # data added without an src) + return old_peak_vertices = self._peak_vertices self._peak_vertices = {} for idx, hemi in enumerate(["lh", "rh", "vol"]): @@ -1245,12 +1250,22 @@ def _update_peak_vertices(self): old_vertex_id = old_peak_vertices.get(hemi) if old_vertex_id == vertex_id: - self._auto_peak_points.add((hemi, vertex_id)) + # same peak vertex but possibly different data: refresh the + # auto-picked trace in place (manually picked traces keep + # showing the overlay they were picked from) + spheres = self._picked_points.get((hemi, vertex_id)) + if (hemi, vertex_id) in self._auto_peak_points and spheres is not None: + spheres[0]["line"].set_ydata( + self._vertex_trace_data(hemi, vertex_id) + ) continue was_auto_picked = (hemi, old_vertex_id) in self._auto_peak_points if old_vertex_id is not None and was_auto_picked: self._remove_vertex_glyph(hemi=hemi, vertex_id=old_vertex_id) - self._auto_peak_points.add((hemi, vertex_id)) + # a vertex the user already picked stays a manual pick (and must + # not be auto-removed on the next overlay switch) + if (hemi, vertex_id) not in self._picked_points: + self._auto_peak_points.add((hemi, vertex_id)) publish( self, VertexSelect(hemi=hemi, vertex_id=vertex_id, source_id=ind[0]), @@ -1258,6 +1273,15 @@ def _update_peak_vertices(self): if self.mpl_canvas is not None: self.mpl_canvas.sync_traces() + def _vertex_trace_data(self, hemi, vertex_id): + """Get the active overlay's time course at a mesh vertex.""" + act_data, smooth = self.act_data_smooth[hemi] + if smooth is not None: + act_data = (smooth[[vertex_id]] @ act_data)[0] + else: # full-resolution data + act_data = act_data[vertex_id].copy() + return act_data + def _update_act_data_smooth(self): # get data for each hemi from scipy.sparse import csr_array @@ -1844,11 +1868,7 @@ def plot_time_course(self, hemi, vertex_id, color, update=True): mni_str = None mni_suffix = "" label = f"{hemi_str}:{str(vertex_id).ljust(6)}{mni_suffix}" - act_data, smooth = self.act_data_smooth[hemi] - if smooth is not None: - act_data = (smooth[[vertex_id]] @ act_data)[0] - else: - act_data = act_data[vertex_id].copy() + act_data = self._vertex_trace_data(hemi, vertex_id) line = self.mpl_canvas.plot( time, act_data, @@ -2289,14 +2309,6 @@ def add_data( self.set_time_interpolation(self.time_interpolation) self._update_colormap_range() - if "data_key" in self.widgets: - keys = list(self._all_data.keys()) - self.widgets["data_key"].set_items(keys) - self.widgets["data_key"].set_value(key) - if len(keys) > 1: - self.widgets["data_key"].show() - self._refresh_colormap_widgets() - # 1) add the surfaces first actor = None for _ in self._iter_views(hemi): @@ -2313,6 +2325,16 @@ def add_data( # _current_time self.set_data_smoothing(self._all_data[key]["smoothing_steps"]) + # setting the data_key widget fires select_data_key, which needs this + # overlay's smooth_mat (and, for volumes, its grid) to already exist + if "data_key" in self.widgets: + keys = list(self._all_data.keys()) + self.widgets["data_key"].set_items(keys) + self.widgets["data_key"].set_value(key) + if len(keys) > 1: + self.widgets["data_key"].show() + self._refresh_colormap_widgets() + # 3) add the other actors if colorbar is True: # bottom left by default diff --git a/mne/viz/_brain/tests/test_brain.py b/mne/viz/_brain/tests/test_brain.py index 0eb6ec4d811..04fb3e3079d 100644 --- a/mne/viz/_brain/tests/test_brain.py +++ b/mne/viz/_brain/tests/test_brain.py @@ -1205,6 +1205,38 @@ def test_brain_overlay_selector(renderer_interactive_pyvistaqt, brain_gc): brain.close() +@testing.requires_testing_data +def test_brain_overlay_switch_label_mode(renderer_interactive_pyvistaqt, brain_gc): + """Overlay switching in label-traces mode must not auto-pick labels. + + Regression: _update_peak_vertices published VertexSelect events in label + mode, toggling the label containing each hemi's peak on every switch (and + crashing outright for overlays added without an src). + """ + brain = _create_testing_brain(hemi="lh", show_traces="label", initial_time=0) + assert brain.traces_mode == "label" + vertices = brain._all_data["data"]["lh"]["vertices"] + time = brain._all_data["data"]["time"] + array2 = np.zeros((len(vertices), len(time))) + array2[0] = 1.0 + brain.add_data( + array2, + fmin=0.0, + fmid=0.5, + fmax=1.0, + vertices=vertices, + time=time, + hemi="lh", + colormap="hot", + key="data2", + smoothing_steps=0, + remove_existing=False, + ) + assert brain._active_data_key == "data2" + assert sum(len(v) for v in brain._picked_patches.values()) == 0 + brain.close() + + @testing.requires_testing_data @pytest.mark.parametrize( "hemi, src", @@ -1543,26 +1575,32 @@ def row_text(row): new_peak_line = next(iter(brain._picked_points.values()))[0]["line"] assert new_peak_line.get_color() == peak_color - # the auto-picked "Peak" trace must follow the active overlay + # the auto-picked "Peak" trace must follow the active overlay; use + # decimated data with the same smoothing as the current widget value so + # that select_data_key cannot rely on a smooth_mat computed as a side + # effect of the smoothing spin box changing (regression: add_data updated + # the data_key widget before set_data_smoothing, crashing on decimated + # overlays) peak1 = brain._peak_vertices["lh"] - n_verts = len(brain.geo["lh"].coords) - peak2 = n_verts - 1 if peak1 != n_verts - 1 else n_verts - 2 + vertices2 = brain._all_data["data"]["lh"]["vertices"] + peak2 = int(vertices2[-1] if peak1 != vertices2[-1] else vertices2[-2]) manual_vertex = next(v for v in (0, 1, 2) if v not in (peak1, peak2)) ui_events.publish(brain, ui_events.VertexSelect(hemi="lh", vertex_id=manual_vertex)) assert ("lh", manual_vertex) in brain._picked_points time = brain._all_data["data"]["time"] - array2 = np.zeros((n_verts, len(time))) - array2[peak2] = 1.0 + array2 = np.zeros((len(vertices2), len(time))) + array2[np.searchsorted(vertices2, peak2)] = 1.0 brain.add_data( array2, fmin=0.0, fmid=0.5, fmax=1.0, - vertices=np.arange(n_verts), + vertices=vertices2, time=time, hemi="lh", colormap="hot", key="data2", + smoothing_steps=0, # match the smoothing widget's current value remove_existing=False, # keep "data" around so we can switch back ) assert brain._active_data_key == "data2" @@ -1586,6 +1624,29 @@ def row_text(row): ) assert row_text(peak_row2) == f"Peak (LH) {peak1}" + # an overlay peaking at the *same* vertex must still refresh the trace + # data (regression: the unchanged-peak branch skipped the re-plot) + peak_line1 = brain._picked_points[("lh", peak1)][0]["line"] + y1 = peak_line1.get_ydata().copy() + brain.add_data( + 2.0 * brain._all_data["data"]["lh"]["array"], + fmin=0.0, + fmid=0.5, + fmax=1.0, + vertices=vertices2, + time=time, + hemi="lh", + colormap="hot", + key="data3", + smoothing_steps=0, + initial_time=0, # match "data" so the peak vertex is the same + remove_existing=False, + ) + assert brain._peak_vertices["lh"] == peak1 + peak_line3 = brain._picked_points[("lh", peak1)][0]["line"] + assert peak_line3 is peak_line1 # refreshed in place, not re-added + assert_allclose(peak_line3.get_ydata(), 2.0 * y1) + def _send_mouse_move(widget, point, buttons=None): """Deliver a synthetic Qt mouse move (QTest.mouseMove warps the real cursor)."""