mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.61k stars 1.3k forks source link

Colorbar labels overlap in Brain plot without time viewer #10131

Open hoechenberger opened 2 years ago

hoechenberger commented 2 years ago

When plotting an STC without the time viewer, the colorbar is arranged horizontally, causing the individual labels to overlap depending on the window size:

# %%
from pathlib import Path
import mne

mne.viz.set_3d_backend('pyvistaqt')
sample_dir = Path(mne.datasets.sample.data_path())
stc_fname = sample_dir / 'MEG' / 'sample' / 'sample_audvis-meg'  # omit suffix
fs_subject = 'sample'
fs_subjects_dir = sample_dir / 'subjects'

stc = mne.read_source_estimate(fname=stc_fname, subject=fs_subject)

stc.plot(
    time_viewer=False,
    size=(300, 300),
    subjects_dir=fs_subjects_dir
)
Screen Shot 2021-12-14 at 17 48 16

I'd suggest to only label the anchors and midpoint.

cc @GuillaumeFavelier

❯ mne sys_info
Platform:       macOS-12.1-x86_64-i386-64bit
Python:         3.9.7 | packaged by conda-forge | (default, Sep 29 2021, 20:33:18)  [Clang 11.1.0 ]
Executable:     /Users/hoechenberger/miniforge3/envs/mne/bin/python3.9
CPU:            i386: 8 cores
Memory:         16.0 GB

mne:            1.0.dev0
numpy:          1.21.4 {blas=NO_ATLAS_INFO, lapack=lapack}
scipy:          1.7.3
matplotlib:     3.5.0 {backend=MacOSX}

sklearn:        1.0.1
numba:          0.53.1
nibabel:        3.2.1
nilearn:        0.8.1
dipy:           1.4.1
cupy:           Not found
pandas:         1.3.4
pyvista:        0.32.1 {OpenGL 4.1 INTEL-18.3.5 via Intel(R) Iris(TM) Plus Graphics OpenGL Engine}
pyvistaqt:      0.5.0
ipyvtklink:     0.2.1
vtk:            9.0.3
PyQt5:          5.12.3
ipympl:         0.8.2
mne_qt_browser: 0.1.7
pooch:          v1.5.2
larsoner commented 2 years ago

I'm not sure if we can come up with a clean internal solution that doesn't create more problems than it solves. Maybe for example we could activate the mode in VTK that tries to choose a size such that labels don't overlap, but then we'll break people's workflows who have explicitly set a size (I think?).

One easy solution is to make clear that you can tweak these things yourself, for example with:

stc.plot(..., add_data_kwargs=dict(
    time_label_size=8,
    colorbar_kwargs=dict(label_font_size=8, n_labels=8)))
larsoner commented 2 years ago

... actually the "auto scaling" in VTK is not for "labels" (what we call "ticks" usually) but rather "annotations". So that's not even an option to try.

I'd just document the ability to tweak these things better somewhere

hoechenberger commented 2 years ago

Can we not simply create some limits based on window size, and decide which labels to show based on that?

Like: small window: only show min, max, mid; large window: show all labels

larsoner commented 2 years ago

Can we not simply create some limits based on window size, and decide which labels to show based on that?

That's one option. (And I don't think it's necessarily trivial to get right -- HiDPI, horizontal vs vertical alignment, number of decimals shown, scientific notation, etc. will all mess with how well this works.) Another would be "just" to adjust the font size so that the same amount of information is shown (rather than more or less based on the window size).

I'm not sure which of these is better, and I'm skeptical that either of them is actually easy/trivial to make work. Hence why I'd rather document that users can control things and let them make the compromises that they see fit with their display(s).

hoechenberger commented 2 years ago

One easy solution is to make clear that you can tweak these things yourself, for example with:

stc.plot(..., add_data_kwargs=dict(
    time_label_size=8,
    colorbar_kwargs=dict(label_font_size=8, n_labels=8)))

This actually does do the trick. However, this API is horribly complicated from a user's perspective (nested dictionaries), given that this problem is quite common (I constantly see it when working with MNE, both on macOS and Linux). Any chance we could agree upon an easier way to achieve this?

larsoner commented 2 years ago

Any chance we could agree upon an easier way to achieve this?

One option would be to make a pyvista.ScalarBarActor class that wraps vtkScalarBarActor with convenient Pythonic access. They give a way of creating an object using nice names:

https://docs.pyvista.org/api/plotting/_autosummary/pyvista.Plotter.add_scalar_bar.html

But then access is all via VTK actor.SetSomethingValue(val) which is not super pythonic. They provide wrappers like this for PolyData so they might be up for it for ScalarBarActor.

drammock commented 2 years ago

this API is horribly complicated from a user's perspective (nested dictionaries)

I think this comment is a bit hyperbolic. Other scientific python libraries do this too (matplotlib's gridspec_kw for example) and we also do this to pass kwargs to ICA fitting functions. I agree that the default behavior is not great, but with better documentation (and maybe better defaults) I'd be OK leaving the API as-is, I don't think it is "horribly complicated".

drammock commented 2 years ago

edit: I didn't look closely enough, just noticed the nested part (colorbar_kwargs inside add_data_kwargs). Agree this should be improved.

hoechenberger commented 2 years ago

Maybe we can discuss some ideas on Friday? :)

Do you see this problem in your daily work? Maybe I'm only looking at particular types of data where this issue becomes visible, and I'm not representative of the common user here?

drammock commented 2 years ago

Maybe we can discuss some ideas on Friday? :)

Sure

Do you see this problem in your daily work?

Not daily, but it does come up, and it does annoy me. Personally I'd also like it to be easy to set the colorbar tick values/locations manually... e.g. if I'm plotting negative log10 of a p-value on the brain, then certain specific values are interesting / important to signpost

larsoner commented 2 years ago

Personally I'd also like it to be easy to set the colorbar tick values/locations manually...

Fun fact, this wasn't even possible in even the dev version of VTK until 9 months ago, mostly (I think?) because ParaView had their own subclass that did something similar. Fortunately it does look like VTK 9.1 included the change that allows doing it:

>>> brain = stc.plot(...)
>>> from vtk.util import numpy_support 
>>> numpy_support.numpy
>>> import numpy as np
>>> brain.plotter.scalar_bar.SetCustomLabels(numpy_support.numpy_to_vtk(np.array([4, 5, 17], float)))
>>> brain.plotter.scalar_bar.SetUseCustomLabels(True)
Screen Shot 2021-12-14 at 7 09 17 PM

To me the best place to make all this accessible would be to subclass vtkScalarBarActor and implement at the PyVista backend as I mentioned above. The vtkScalarBarActor is currently accessible via brain.plotter.scalar_bar and brain._scalar_bar, so I say we change it to brain.scalar_bar (public name) and then make brain.scalar_bar have nice methods at the PyVista level. It's much more in PyVista's scope to provide convenience wrappers for VTK "stuff" than MNE-Python, so I'd rather work to solve the problem / implement a solution at their end with the help of their expertise!