Passing backend='pyvista' to plot_source_estimates() currently is not supported, although we do support PyVista:
https://github.com/mne-tools/mne-python/blob/6f16c3771d79dda19b5ff0a058396f468a3e57f6/mne/viz/_3d.py#L1750
Also, the backend parameter is not used for anything except for checking whether we should use Matplotlib or not; if backend != 'matplotlib', we simply use _get_3d_backend() to retrieve the backend to use. This means that e.g.if you have PyVista installed, passing backend='mayavi' will still use the pyvista backend.
backend should be allowed to be 'pyvista' as wellbackend should be honoredAlso wondering if the backend param should be deprecated or changed. Maybe it could only support matplotlib and 3d or so, and 3d would select the default 3D backend?
Or we deprecate it and if we cannot detect a valid 3D backend, we fall back to Matplotlib. (We already kind of do that, if I'm reading the code correctly)
cc @GuillaumeFavelier
Good catch @hoechenberger !
Or we deprecate it and if we cannot detect a valid 3D backend, we fall back to Matplotlib.
+1 for checking if any 3d backend is available and use matplotlib as fallback.
We already have set_3d_backend() and use_3d_backend() to set mayavi as backend so in my opinion this is redundant.
What do you think @agramfort, @larsoner ?
agreed !
Ok so:
backend paramIs that what we want? Are there maybe use cases where one does have a working 3D backend but might want to use MPL anyway? (for love of pixelated graphics?)
+100 to get rid of the matplotlib backend altogether and tell people to use nilearn's surface plotting and show how to do it in plot_visualize_stc if they want to use matplotlib
not too fast.
what I have seen in the past is:
why not the nilearn option (I never used it myself) but I must be certain
it just works like the matplotlib version. It should work on google collab
too as it's what most students use on hands on session with master students.
>
Yes I would also like us to be careful about removing MPL support. It's a great fallback to have when we're using the MNE Study Template on a remote server without X11.
indeed we use it in the study template. So let's not break this.
>
why not the nilearn option (I never used it myself) but I must be certain it just works like the matplotlib version.
If we're going to keep it, we should probably make sure it actually works, and add it to some examples like we do for the rest of our code. I would be surprised if all combinations of transparent, lims, pos_lims, surface, etc. work correctly for the matplotlib backend like they do for the 3D one(s). In other words, I'm worried that the matplotlib version "works" in the sense that it gives an output, but that output is sometimes incorrect because that code has been as far as I've seen mostly unchanged for years (despite improvements and fixes to our STC code for the 3D backends).
...and I think moving to nilearn hopefully would put some of the "make sure it works" burden onto the nilearn folks rather than us -- hence the motivation for at least using their stuff under the hood. But if adding nilearn as a dependency to get plotting is too much (it's not just NumPy/SciPy/matplotlib) then we should probably take on the maintenance burden.
Let's put it another way:
I really don't care WHAT the backend is, as long as I can use it to create a basic STC plot even if I'm not on a machine with 3D support. If we can find a solution for that, I'm good!
nilearn is easy to install so it's a possibility I can endorse but I need
to make sure it works smoothly, typically in google collab
I hear that options may be ignored with matplotlib but with default
parameters it works
>
@larsoner
+100 to get rid of the matplotlib backend altogether and tell people to use nilearn's surface plotting and show how to do it in
plot_visualize_stcif they want to use matplotlib
We should not only provide an example but it should be as convenient to use as the current MPL backend, IMHO, even if only a small set of the functionality we offer for 3D backends is supported.
But to move forward for now, we should merge #8395, as it fixes two bugs :)