MNT: notebook conditionals in Brain & _update

[GuillaumeFavelier]

This PR follows #9014 (comment) and refactors some conditionals out of Brain.

It also helps with #8997

[larsoner]

Looks like it's improved but not complete:

$ git grep "if self\.notebook"
mne/viz/_brain/_brain.py:        if self.notebook:
mne/viz/_brain/_brain.py:        if self.notebook:
mne/viz/_brain/_brain.py:        if self.notebook:
mne/viz/_brain/_brain.py:        if self.notebook:
mne/viz/_brain/_brain.py:        if self.notebook:
mne/viz/_brain/callback.py:        if self.notebook:
mne/viz/_brain/callback.py:        if self.notebook:
mne/viz/_brain/mplcanvas.py:        if self.notebook:
mne/viz/_brain/mplcanvas.py:        if self.notebook:
mne/viz/backends/_pyvista.py:        if self.notebook:

Or are these too difficult to refactor?

[GuillaumeFavelier]

I take the time to do it properly. Some of them are not obvious, for example:

mne-python/mne/viz/_brain/_brain.py

Lines 667 to 678 in 0f0fa02

	if self.notebook:
	self._renderer.show()
	self.mpl_canvas.show()
	self.toggle_interface()
	if not self.notebook:
	self._configure_playback()
	# show everything at the end
	with _qt_disable_paint(self.plotter):
	with self._ensure_minimum_sizes():
	self.show()
	self._renderer._update()

and

mne-python/mne/viz/_brain/mplcanvas.py

Lines 32 to 48 in 0f0fa02

	if self.notebook:
	from matplotlib.backends.backend_nbagg import (FigureCanvasNbAgg,
	FigureManager)
	self.canvas = FigureCanvasNbAgg(self.fig)
	self.manager = FigureManager(self.canvas, 0)
	else:
	from PyQt5 import QtWidgets
	from matplotlib.backends.backend_qt5agg import FigureCanvasQTAgg
	self.canvas = FigureCanvasQTAgg(self.fig)
	self.canvas.setParent(parent)
	FigureCanvasQTAgg.setSizePolicy(
	self.canvas,
	QtWidgets.QSizePolicy.Expanding,
	QtWidgets.QSizePolicy.Expanding
	)
	FigureCanvasQTAgg.updateGeometry(self.canvas)
	self.manager = None

I might have an idea for window, interactor and _screenshot.

Those in callback.py are there by design. Removing them would require something like _AbstractWidget/_IpyWidget/_QtWidget I think.

[larsoner]

Looks like great progress! It also seems like the last few if self.notebooks in Brain could be pushed into the self._renderer methods, since they involve using either the notebook screenshot filename + notebook size or Qt file dialog + Qt interactor size.

I looked at mplcanvas.py, maybe the two _renderer flavors could also each have a _matplotlib_canvas() function? Then the ipy matplotlib canvas + config lives in the _notebook.py file, and the Qt5Agg canvas + config lives in the _qt.py file.

[GuillaumeFavelier]

I lack inspiration to get rid of self.interactor completely so I tried b819d02. This can probably be improved.

[larsoner]

I could 8 .notebooks in _brain.py. I do think most of them can still be refactored into the Qt and notebook files, as well as the conditionals in mplcanvas.py (probably by making an AbstractMplCanvas). Do you want to do it in this PR, or another PR (for you or me)?

[GuillaumeFavelier]

I am waiting for the CIs, hopefully the failure on MacOS is just random. I tried to refactor the last bit of:

mne-python/mne/viz/_brain/_brain.py

Lines 659 to 664 in 62e29ed

	if self.notebook:
	self._renderer.show()
	else:
	with _qt_disable_paint(self.plotter):
	with self._ensure_minimum_sizes():
	self.show()

into:

mne-python/mne/viz/_brain/_brain.py

Lines 658 to 665 in 98bbc62

	self._renderer._window_show(self._size)
	# sizes could change, update views
	for hemi in ('lh', 'rh'):
	for ri, ci, v in self._iter_views(hemi):
	self.show_view(view=v, row=ri, col=ci)
	self._renderer._process_events()

But this part requires double-checking.

This is ready for reviews @larsoner

[larsoner]

Awesome!

$ git grep "\.notebook"
mne/externals/tqdm/_tqdm/__init__.py:    """See tqdm.notebook.tqdm for full documentation"""
...
mne/externals/tqdm/_tqdm/notebook.py:    A shortcut for `tqdm.notebook.tqdm(xrange(*args), **kwargs)`.
mne/viz/_brain/_brain.py:        self.notebook = (_get_3d_backend() == "notebook")
mne/viz/_brain/tests/test.ipynb:    "    assert brain.notebook\n",
mne/viz/backends/_pyvista.py:        self.notebook = notebook
mne/viz/backends/_pyvista.py:        if not self.notebook:
mne/viz/backends/_pyvista.py:        if self.notebook:
mne/viz/backends/_pyvista.py:            if not self.notebook:
mne/viz/backends/_pyvista.py:            if not self.notebook and hasattr(plotter_class, 'set_icon'):

I would get rid of the brain.notebook property altogether at this point. In the ipynb test I would check for notebook-ness some other way (correct isinstance of something?)

[larsoner]

Let's merge once CIs come back happy, thanks in advance @GuillaumeFavelier !