Backport PR #13334: MAINT: Dont set private attributes for PyVista Actor

Author: larsoner

.circleci/config.yml

@@ -93,11 +93,6 @@ jobs:
           name: Set BASH_ENV
           command: ./tools/circleci_bash_env.sh
 
-      - run:
-          name: check neuromag2ft
-          command: |
-            neuromag2ft --version
-
       - run:
           name: Install fonts needed for diagrams
           command: |

mne/gui/_coreg.py

@@ -292,6 +292,7 @@ def _get_default(var, val):
         self._renderer.set_interaction(interaction)
 
         # coregistration model setup
+        self._picking_targets = list()
         self._immediate_redraw = self._renderer._kind != "qt"
         self._info = info
         self._fiducials = fiducials
@@ -899,10 +900,10 @@ def _on_button_release(self, vtk_picker, event):
         if self._mouse_no_mvt > 0:
             x, y = vtk_picker.GetEventPosition()
             # XXX: internal plotter/renderer should not be exposed
-            plotter = self._renderer.figure.plotter
+            picker = self._renderer._picker
             picked_renderer = self._renderer.figure.plotter.renderer
             # trigger the pick
-            plotter.picker.Pick(x, y, 0, picked_renderer)
+            picker.Pick(x, y, 0, picked_renderer)
         self._mouse_no_mvt = 0
 
     def _on_pick(self, vtk_picker, event):
@@ -913,7 +914,7 @@ def _on_pick(self, vtk_picker, event):
         mesh = vtk_picker.GetDataSet()
         if mesh is None or cell_id == -1 or not self._mouse_no_mvt:
             return
-        if not getattr(mesh, "_picking_target", False):
+        if not any(mesh is target for target in self._picking_targets):
             return
         pos = np.array(vtk_picker.GetPickPosition())
         vtk_cell = mesh.GetCell(cell_id)
@@ -1341,7 +1342,7 @@ def _add_head_surface(self):
             key = "low"
         self._update_actor("head", head_actor)
         # mark head surface mesh to restrict picking
-        head_surf._picking_target = True
+        self._picking_targets.append(head_surf)
         # We need to use _get_processed_mri_points to incorporate grow_hair
         rr = self.coreg._get_processed_mri_points(key) * self.coreg._scale.T
         head_surf.points = rr

mne/viz/_brain/_brain.py

@@ -535,10 +535,8 @@ def setup_time_viewer(self, time_viewer=True, show_traces=True):
         self.mpl_canvas = None
         self.help_canvas = None
         self.rms = None
-        self.picked_patches = {key: list() for key in all_keys}
-        self.picked_points = {key: list() for key in all_keys}
-        self.pick_table = dict()
-        self._spheres = list()
+        self._picked_patches = {key: list() for key in all_keys}
+        self._picked_points = dict()
         self._mouse_no_mvt = -1
 
         # Derived parameters:
@@ -622,6 +620,8 @@ def _clean(self):
         self.plotter._Iren = _FakeIren()
         if getattr(self.plotter, "picker", None) is not None:
             self.plotter.picker = None
+        if getattr(self._renderer, "_picker", None) is not None:
+            self._renderer._picker = None
         # XXX end PyVista
         for key in (
             "plotter",
@@ -925,7 +925,7 @@ def _set_annot(annot):
         def _set_label_mode(mode):
             if self.traces_mode != "label":
                 return
-            glyphs = copy.deepcopy(self.picked_patches)
+            glyphs = copy.deepcopy(self._picked_patches)
             self.label_extract_mode = mode
             self.clear_glyphs()
             for hemi in self._hemis:
@@ -1048,7 +1048,7 @@ def _configure_vertex_time_course(self):
             # simulate a picked renderer
             if self._hemi in ("both", "rh") or hemi == "vol":
                 idx = 0
-            self.picked_renderer = self._renderer._all_renderers[idx]
+            self._picked_renderer = self._renderer._all_renderers[idx]
 
             # initialize the default point
             if self._data["initial_time"] is not None:
@@ -1201,16 +1201,9 @@ def _on_button_release(self, vtk_picker, event):
         if self._mouse_no_mvt > 0:
             x, y = vtk_picker.GetEventPosition()
             # programmatically detect the picked renderer
-            try:
-                # pyvista<0.30.0
-                self.picked_renderer = self.plotter.iren.FindPokedRenderer(x, y)
-            except AttributeError:
-                # pyvista>=0.30.0
-                self.picked_renderer = self.plotter.iren.interactor.FindPokedRenderer(
-                    x, y
-                )
+            self._picked_renderer = self.plotter.iren.interactor.FindPokedRenderer(x, y)
             # trigger the pick
-            self.plotter.picker.Pick(x, y, 0, self.picked_renderer)
+            self._renderer._picker.Pick(x, y, 0, self.picked_renderer)
         self._mouse_no_mvt = 0
 
     def _on_pick(self, vtk_picker, event):
@@ -1224,28 +1217,17 @@ def _on_pick(self, vtk_picker, event):
         if mesh is None or cell_id == -1 or not self._mouse_no_mvt:
             return  # don't pick
 
-        # 1) Check to see if there are any spheres along the ray
-        if len(self._spheres):
+        # 1) Check to see if there are any spheres along the ray and remove if so
+        if len(self._picked_points):
             collection = vtk_picker.GetProp3Ds()
-            found_sphere = None
             for ii in range(collection.GetNumberOfItems()):
                 actor = collection.GetItemAsObject(ii)
-                for sphere in self._spheres:
-                    if any(a is actor for a in sphere._actors):
-                        found_sphere = sphere
-                        break
-                if found_sphere is not None:
-                    break
-            if found_sphere is not None:
-                assert found_sphere._is_glyph
-                mesh = found_sphere
-
-        # 2) Remove sphere if it's what we have
-        if hasattr(mesh, "_is_glyph"):
-            self._remove_vertex_glyph(mesh)
-            return
+                for (hemi, vertex_id), spheres in self._picked_points.items():
+                    if any(sphere["actor"] is actor for sphere in spheres):
+                        self._remove_vertex_glyph(hemi=hemi, vertex_id=vertex_id)
+                        return
 
-        # 3) Otherwise, pick the objects in the scene
+        # 2) Otherwise, pick the objects in the scene
         for hemi, this_mesh in self._layered_meshes.items():
             assert hemi in ("lh", "rh"), f"Unexpected {hemi=}"
             if this_mesh._polydata is mesh:
@@ -1359,24 +1341,25 @@ def _add_label_glyph(self, hemi, mesh, vertex_id):
         label = self._annotation_labels[hemi][label_id]
 
         # remove the patch if already picked
-        if label_id in self.picked_patches[hemi]:
+        if label_id in self._picked_patches[hemi]:
             self._remove_label_glyph(hemi, label_id)
             return
 
         if hemi == label.hemi:
             self.add_label(label, borders=True)
-            self.picked_patches[hemi].append(label_id)
+            self._picked_patches[hemi].append(label_id)
 
     def _remove_label_glyph(self, hemi, label_id):
         label = self._annotation_labels[hemi][label_id]
         label._line.remove()
         self.color_cycle.restore(label._color)
         self.mpl_canvas.update_plot()
         self._layered_meshes[hemi].remove_overlay(label.name)
-        self.picked_patches[hemi].remove(label_id)
+        self._picked_patches[hemi].remove(label_id)
 
     def _add_vertex_glyph(self, hemi, mesh, vertex_id, update=True):
-        if vertex_id in self.picked_points[hemi]:
+        _ensure_int(vertex_id)
+        if (hemi, vertex_id) in self._picked_points:
             return
 
         # skip if the wrong hemi is selected
@@ -1400,10 +1383,9 @@ def _add_vertex_glyph(self, hemi, mesh, vertex_id, update=True):
             lst = self._renderer._all_renderers._renderers
         except AttributeError:
             lst = self._renderer._all_renderers
-        rindex = lst.index(self.picked_renderer)
+        rindex = lst.index(self._picked_renderer)
         row, col = self._renderer._index_to_loc(rindex)
 
-        actors = list()
         spheres = list()
         for _ in self._iter_views(hemi):
             # Using _sphere() instead of renderer.sphere() for 2 reasons:
@@ -1412,39 +1394,28 @@ def _add_vertex_glyph(self, hemi, mesh, vertex_id, update=True):
             #    mitigated with synchronization/delay?)
             # 2) the glyph filter is used in renderer.sphere() but only one
             #    sphere is required in this function.
-            actor, sphere = self._renderer._sphere(
+            actor, mesh = self._renderer._sphere(
                 center=np.array(center),
                 color=color,
                 radius=4.0,
             )
-            actors.append(actor)
-            spheres.append(sphere)
+            spheres.append(dict(mesh=mesh, actor=actor))
 
         # add metadata for picking
         for sphere in spheres:
-            sphere._is_glyph = True
-            sphere._hemi = hemi
-            sphere._line = line
-            sphere._actors = actors
-            sphere._color = color
-            sphere._vertex_id = vertex_id
-
-        self.picked_points[hemi].append(vertex_id)
-        self._spheres.extend(spheres)
-        self.pick_table[vertex_id] = spheres
-        return sphere
+            sphere.update(hemi=hemi, line=line, color=color, vertex_id=vertex_id)
 
-    def _remove_vertex_glyph(self, mesh, render=True):
-        vertex_id = mesh._vertex_id
-        if vertex_id not in self.pick_table:
-            return
+        _ensure_int(vertex_id)
+        self._picked_points[(hemi, vertex_id)] = spheres
+        return sphere
 
-        hemi = mesh._hemi
-        color = mesh._color
-        spheres = self.pick_table[vertex_id]
-        spheres[0]._line.remove()
+    def _remove_vertex_glyph(self, *, hemi, vertex_id, render=True):
+        _ensure_int(vertex_id)
+        assert isinstance(hemi, str), f"got {type(hemi)} for {hemi=}"
+        spheres = self._picked_points.pop((hemi, vertex_id))
+        color, line = spheres[0]["color"], spheres[0]["line"]
+        line.remove()
         self.mpl_canvas.update_plot()
-        self.picked_points[hemi].remove(vertex_id)
 
         with warnings.catch_warnings(record=True):
             # We intentionally ignore these in case we have traversed the
@@ -1453,26 +1424,21 @@ def _remove_vertex_glyph(self, mesh, render=True):
             self.color_cycle.restore(color)
         for sphere in spheres:
             # remove all actors
-            self.plotter.remove_actor(sphere._actors, render=False)
-            sphere._actors = None
-            self._spheres.pop(self._spheres.index(sphere))
+            self.plotter.remove_actor(sphere.pop("actor"), render=False)
         if render:
             self._renderer._update()
-        self.pick_table.pop(vertex_id)
 
     def clear_glyphs(self):
         """Clear the picking glyphs."""
         if not self.time_viewer:
             return
-        for sphere in list(self._spheres):  # will remove itself, so copy
-            self._remove_vertex_glyph(sphere, render=False)
-        assert sum(len(v) for v in self.picked_points.values()) == 0
-        assert len(self.pick_table) == 0
-        assert len(self._spheres) == 0
+        for hemi, vertex_id in list(self._picked_points):
+            self._remove_vertex_glyph(hemi=hemi, vertex_id=vertex_id, render=False)
+        assert len(self._picked_points) == 0
         for hemi in self._hemis:
-            for label_id in list(self.picked_patches[hemi]):
+            for label_id in list(self._picked_patches[hemi]):
                 self._remove_label_glyph(hemi, label_id)
-        assert sum(len(v) for v in self.picked_patches.values()) == 0
+        assert sum(len(v) for v in self._picked_patches.values()) == 0
         if self.rms is not None:
             self.rms.remove()
             self.rms = None
@@ -4025,11 +3991,14 @@ def get_picked_points(self):
 
         Returns
         -------
-        points : list of int | None
-            The vertices picked by the time viewer.
+        points : dict | None
+            The vertices picked by the time viewer, one key per hemisphere with
+            a list of vertex indices.
         """
-        if hasattr(self, "time_viewer"):
-            return self.picked_points
+        out = dict(lh=[], rh=[], vol=[])
+        for hemi, vertex_id in self._picked_points:
+            out[hemi].append(vertex_id)
+        return out
 
     def __hash__(self):
         """Hash the object."""

mne/viz/_brain/_linkviewer.py

@@ -49,7 +49,7 @@ def _func_remove(*args, **kwargs):
             for hemi in ("lh", "rh"):
                 initial_points[hemi] = set()
                 for brain in self.brains:
-                    initial_points[hemi] |= set(brain.picked_points[hemi])
+                    initial_points[hemi] |= set(brain.get_picked_points()[hemi])
 
             # link the viewers
             for brain in self.brains:

mne/viz/_brain/tests/test_brain.py

@@ -1013,19 +1013,19 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
             current_mesh = brain._layered_meshes[current_hemi]._polydata
             cell_id = rng.randint(0, current_mesh.n_cells)
             test_picker = TstVTKPicker(current_mesh, cell_id, current_hemi, brain)
-            assert len(brain.picked_patches[current_hemi]) == 0
+            assert len(brain._picked_patches[current_hemi]) == 0
             brain._on_pick(test_picker, None)
-            assert len(brain.picked_patches[current_hemi]) == 1
-            for label_id in list(brain.picked_patches[current_hemi]):
+            assert len(brain._picked_patches[current_hemi]) == 1
+            for label_id in list(brain._picked_patches[current_hemi]):
                 label = brain._annotation_labels[current_hemi][label_id]
                 assert isinstance(label._line, Line2D)
             brain.widgets["extract_mode"].set_value("mean")
             brain.clear_glyphs()
-            assert len(brain.picked_patches[current_hemi]) == 0
+            assert len(brain._picked_patches[current_hemi]) == 0
             brain._on_pick(test_picker, None)  # picked and added
-            assert len(brain.picked_patches[current_hemi]) == 1
+            assert len(brain._picked_patches[current_hemi]) == 1
             brain._on_pick(test_picker, None)  # picked again so removed
-            assert len(brain.picked_patches[current_hemi]) == 0
+            assert len(brain._picked_patches[current_hemi]) == 0
         # test switching from 'label' to 'vertex'
         brain.widgets["annotation"].set_value("None")
         brain.widgets["extract_mode"].set_value("max")
@@ -1067,8 +1067,7 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
     )
     assert brain.show_traces
     assert brain.traces_mode == "vertex"
-    assert hasattr(brain, "picked_points")
-    assert hasattr(brain, "_spheres")
+    assert hasattr(brain, "_picked_points")
     assert brain._scalar_bar.GetNumberOfLabels() == 3
 
     # add foci should work for 'lh', 'rh' and 'vol'
@@ -1078,7 +1077,7 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
 
     # test points picked by default
     picked_points = brain.get_picked_points()
-    spheres = brain._spheres
+    spheres = sum(brain._picked_points.values(), list())
     for current_hemi in hemi_str:
         assert len(picked_points[current_hemi]) == 1
     n_spheres = len(hemi_str)
@@ -1096,7 +1095,9 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
         brain.widgets["annotation"].set_value("None")
     # test removing points
     brain.clear_glyphs()
+    spheres = sum(brain._picked_points.values(), list())
     assert len(spheres) == 0
+    picked_points = brain.get_picked_points()
     for key in ("lh", "rh", "vol"):
         assert len(picked_points[key]) == 0
 
@@ -1120,13 +1121,16 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
         brain._on_pick(test_picker, None)
         brain._on_pick(test_picker, None)
         assert test_picker.point_id is not None
+        picked_points = brain.get_picked_points()
         assert len(picked_points[current_hemi]) == 1
         assert picked_points[current_hemi][0] == test_picker.point_id
+        spheres = sum(brain._picked_points.values(), list())
         assert len(spheres) > 0
         sphere = spheres[-1]
-        vertex_id = sphere._vertex_id
+        vertex_id = sphere["vertex_id"]
         assert vertex_id == test_picker.point_id
-        line = sphere._line
+        line = sphere["line"]
+        del sphere
 
         hemi_prefix = current_hemi[0].upper()
         if current_hemi == "vol":
@@ -1148,8 +1152,9 @@ def test_brain_traces(renderer_interactive_pyvistaqt, hemi, src, tmp_path, brain
 
         # remove the sphere by clicking in its vicinity
         old_len = len(spheres)
-        test_picker._actors = sum((s._actors for s in spheres), [])
+        test_picker._actors = [s["actor"] for s in spheres]
         brain._on_pick(test_picker, None)
+        spheres = sum(brain._picked_points.values(), list())
         assert len(spheres) < old_len
 
     screenshot = brain.screenshot()
@@ -1387,7 +1392,7 @@ def test_brain_ui_events(renderer_interactive_pyvistaqt, brain_gc):
     assert brain._current_time == 1
 
     ui_events.publish(brain, ui_events.VertexSelect(hemi="lh", vertex_id=1))
-    assert 1 in brain.picked_points["lh"]
+    assert 1 in brain.get_picked_points()["lh"]
 
     ui_events.publish(
         brain,