Skip to content

MRG: Add support for foreground in _Brain#7843

Merged
larsoner merged 4 commits intomne-tools:masterfrom
GuillaumeFavelier:brain_foreground_color
May 28, 2020
Merged

MRG: Add support for foreground in _Brain#7843
larsoner merged 4 commits intomne-tools:masterfrom
GuillaumeFavelier:brain_foreground_color

Conversation

@GuillaumeFavelier
Copy link
Copy Markdown
Contributor

@GuillaumeFavelier GuillaumeFavelier commented May 28, 2020

This PR adds support for the foreground parameter in _Brain. This parameter influences the color of the labels displayed (scalar bar label, time label and slider title and label).

Following the example given in #7842 (comment), this is what I obtain now:

time_viewer=False time_viewer=True
image image

Closes #7842
It's an item of #7162

@GuillaumeFavelier GuillaumeFavelier self-assigned this May 28, 2020
@GuillaumeFavelier GuillaumeFavelier mentioned this pull request May 28, 2020
Closed
86 tasks
agramfort
agramfort reviewed May 28, 2020
View reviewed changes
mne/viz/backends/base_renderer.py
The color of the label text.
title: str | None
The title of the scalar bar.
n_labels: int | None
Copy link
Copy Markdown
Member

@agramfort agramfort May 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bgcolor not documented?

@GuillaumeFavelier GuillaumeFavelier changed the title Add support for foreground in _Brain MRG: Add support for foreground in _Brain May 28, 2020
larsoner
larsoner reviewed May 28, 2020
View reviewed changes
mne/viz/_brain/_brain.py Outdated
if isinstance(background, str):
background = colorConverter.to_rgb(background)
if foreground is None:
foreground = "white"
Copy link
Copy Markdown
Member

@larsoner larsoner May 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should set it to white or black automatically depending on the luminance of the background, see how it's done in PySurfer

@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 28, 2020

Can you add to the TODO list to make the matplotlib figure use the same foreground/background colors as the plotter above it? It seems like they should match...

@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 28, 2020

(not necessarily for this PR)

@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 28, 2020 •
edited
Loading

Pushed a commit to fix pyvistaqt and to fix our default to foreground=None

I did notice a bug with the VectorSourceEstimate plotting -- it looks like the colormap of the arrows are not interactively updated, see how the brain surface and arrow colormaps do not match here:

Screenshot from 2020-05-28 10-41-39

Sample code to reproduce, just click around the sliders and you should see the brain surface change color but the color/alpha of the arrows stays the same:

Sample code 
import os.path as op
import mne

data_path = mne.datasets.sample.data_path()
sample_dir = op.join(data_path, 'MEG', 'sample')
subjects_dir = op.join(data_path, 'subjects')
inv = mne.minimum_norm.read_inverse_operator(op.join(
    sample_dir, 'sample_audvis-meg-oct-6-meg-inv.fif'))
evoked = mne.read_evokeds(op.join(sample_dir, 'sample_audvis-ave.fif'))[0]
evoked.apply_baseline((None, 0))
stc = mne.minimum_norm.apply_inverse(
    evoked, inv, method='dSPM', verbose='debug', pick_ori='vector')  # 'vector')
initial_time = 0.1

size = (800, 600)
with mne.viz.use_3d_backend('pyvista'):
    brain = stc.plot(subjects_dir=subjects_dir, initial_time=initial_time,
                     clim='auto', views='lat', hemi='lh',
                     smoothing_steps='nearest', verbose=True,
                     size=size, background='w')

It becomes even more obvious if you set something like clim=dict(kind='value', lims=(25, 27, 29)) then change the limits to be lower, the brain lights up but the arrows do not show up.

@GuillaumeFavelier
Copy link
Copy Markdown
Contributor Author

GuillaumeFavelier commented May 28, 2020

I updated #7162 with #7843 (comment) and #7843 (comment)

@agramfort
Copy link
Copy Markdown
Member

agramfort commented May 28, 2020 via email

@larsoner larsoner merged commit bbeb2ae into mne-tools:master May 28, 2020
@larsoner
Copy link
Copy Markdown
Member

larsoner commented May 28, 2020

Thanks @GuillaumeFavelier

larsoner added a commit to larsoner/mne-python that referenced this pull request May 28, 2020
@larsoner
Merge remote-tracking branch 'upstream/master' into refactor
755acfb 
* upstream/master:
  MRG: Add support for foreground in _Brain (mne-tools#7843)
  MRG, MAINT: Change default role in conf.py (mne-tools#7841)
  MRG, ENH: Support n_col keyword in ica.plot_score (mne-tools#7825)
  add icons to source dist (mne-tools#7840)
  Add CZI to list of funders (mne-tools#7839)
  DOC: added reference to sesameeg package (mne-tools#7835)
  MRG, ENH: Automatically compute threshold for CTPS ECG detection (mne-tools#7819)
  MAINT: Show how picks work for planars (mne-tools#7833)
  Clearer info docstring (mne-tools#7832)
  MRG, ENH: Add estimation method legend (mne-tools#7830)
  Remove double spaces (mne-tools#7822)
  add troubleshooting message about OpenGL [skip travis] (mne-tools#7827)
  fix links [skip travis] (mne-tools#7826)
@GuillaumeFavelier GuillaumeFavelier deleted the brain_foreground_color branch June 11, 2020 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@agramfort agramfort agramfort left review comments

@larsoner larsoner larsoner left review comments

Assignees

@GuillaumeFavelier GuillaumeFavelier

Labels

VIZ

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Pyvista foreground bug

3 participants

@GuillaumeFavelier @larsoner @agramfort