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?

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

and

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.

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.

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

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)?

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

into:

But this part requires double-checking.

This is ready for reviews @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?)