Mne-python: backend parameter of viz._3d.plot_source_estimates needs some love

Created on 21 Oct 2020  路  13Comments  路  Source: mne-tools/mne-python

Describe the bug

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.

https://github.com/mne-tools/mne-python/blob/6f16c3771d79dda19b5ff0a058396f468a3e57f6/mne/viz/_3d.py#L1751-L1762

Expected results

  • backend should be allowed to be 'pyvista' as well
  • the value of backend should be honored

Additional information

Also 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

API BUG VIZ

All 13 comments

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:

  • we deprecate the backend param
  • use Matplotlib only if we cannot retrieve a valid 3D backend

Is 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:

  • students with a broken mayavi (core dump on import). Passing matplotlib
    as explicit backend fixed it. Otherwise mne would crash.
  • when I teach intro to mne for young grad students I tell them install
    numpy, scipy and matplotlib. It's all they need to get started and do
    things. I cannot tell them to install a conda env at the beginning of my
    class.

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_stc if 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 :)

Was this page helpful?
0 / 5 - 0 ratings