Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MRG: Add support for foreground in _Brain #7843

Merged
merged 4 commits into from
May 28, 2020
Merged

MRG: Add support for foreground in _Brain #7843

merged 4 commits into from
May 28, 2020

Conversation

GuillaumeFavelier
Copy link
Contributor

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
"""Add a scalar bar in the scene.

Parameters
----------
source:
The object of the scene used for the colormap.
color:
The color of the label text.
title: str | None
The title of the scalar bar.
n_labels: int | None
Copy link
Member

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
@@ -177,6 +177,8 @@ def __init__(self, subject_id, hemi, surf, title=None,

if isinstance(background, str):
background = colorConverter.to_rgb(background)
if foreground is None:
foreground = "white"
Copy link
Member

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
Member

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
Member

(not necessarily for this PR)

@larsoner
Copy link
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
Contributor Author

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

@agramfort
Copy link
Member

agramfort commented May 28, 2020 via email

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

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