Dark-mode to PyVista-Brain (pair-programmed with @GuillaumeFavelier) #9149

marsipu · 2021-03-18T15:07:26Z

This follows PR #9126.

What does this implement/fix?

Add a theme-parameter to Brain with PyVista-Renderer accepting 'dark', 'light' and 'auto'. Dark-Mode depends on qdarkstyle and the automatic detection works with darkdetect.

larsoner · 2021-03-18T15:13:50Z

mne/viz/_brain/_brain.py

+        to style the widgets. Automatic detection does not yet work on
+        Linux. Can also be a path-like to a custom style sheet.


I would open an upstream issue with darkdetect for this if they don't have one. In the meantime I would use:

gsettings get org.gnome.desktop.interface gtk-theme

if it ends in -dark assume it's dark. It at least works on my machine™ :)

@cbrnr opened an issue

This one: albertosottile/darkdetect#7

mne/viz/_brain/_brain.py

mne/viz/backends/_qt.py

GuillaumeFavelier · 2021-03-18T16:45:28Z

@marsipu I pushed a commit to reorder theme and take #9149 (comment) into account

marsipu · 2021-03-18T16:59:23Z

@marsipu I pushed a commit to reorder theme and take #9149 (comment) into account

Thank you @GuillaumeFavelier . I also thought we maybe could shorten the documentation-text a bit more like this?

GuillaumeFavelier · 2021-03-18T17:13:35Z

we maybe could shorten the documentation-text a bit more like this?

Sounds good 👍

GuillaumeFavelier · 2021-03-18T17:19:30Z

mne/viz/_brain/_brain.py

+    theme : str | path-like
+        Can be "auto" (default), "light", or "dark" or a path-like to a
+        custom stylesheet. For Dark-Mode and automatic Dark-Mode-Detection,
+        `qdarkstyle` respectively `darkdetect` is required.


CircleCI seems to have some issues with the new modules:

/home/circleci/project/mne/viz/_brain/_brain.py:docstring of mne.viz._brain._brain.Brain:97: WARNING: py:obj reference target not found: qdarkstyle /home/circleci/project/mne/viz/_brain/_brain.py:docstring of mne.viz._brain._brain.Brain:97: WARNING: py:obj reference target not found: darkdetect

Probably the docstring needs to be updated:

Suggested change

`qdarkstyle` respectively `darkdetect` is required.

:mod:`qdarkstyle` respectively :mod:`darkdetect` is required.

And the list intersphinx_mapping in doc/conf.py too.

marsipu · 2021-03-18T18:25:55Z

Somehow when I test it locally, the :mod:-phrases doesn't turn into a link leading to the websites. It is supposed to do that right? Do I maybe need to import the intersphinx-references into `_brain.py´?

larsoner · 2021-03-18T19:15:33Z

The :mod: should not be necessary I think. The problem is probably more that these modules are not in our intersphinx inventory in doc/conf.py.

https://qdarkstylesheet.readthedocs.io/en/latest/reference/qdarkstyle.html

I don't see a Sphinx documentation page for darkdetect so I would just make that one code with double-backticks rather than try to link.

larsoner · 2021-03-18T19:15:51Z

(or you could do a custom HTML link to the darkdetect pypi or GitHub page if you want)

doc/conf.py

mne/viz/_brain/_brain.py

marsipu · 2021-03-19T12:03:54Z

The failed test in mne/decoding/tests/test_transformer.py::test_scaler is probably unrelated, right?

GuillaumeFavelier · 2021-03-19T16:10:05Z

This is what I saw:

=================================== FAILURES ===================================
_________________________________ test_scaler __________________________________
mne/decoding/tests/test_transformer.py:67: in test_scaler
    assert_allclose(X * stds[:, np.newaxis] + means[:, np.newaxis],
E   AssertionError: 
E   Not equal to tolerance rtol=1e-12, atol=1e-20
E   mean
E   Mismatched elements: 23576 / 23576 (100%)
E   Max absolute difference: 2.00587641e-11
E   Max relative difference: 42.17865626
E    x: array([[[-1.005809e-13, -1.005809e-13, -1.005809e-13, ...,
E            -1.005809e-13, -1.005809e-13, -1.005809e-13],
E           [ 2.151313e-13,  2.151313e-13,  2.151313e-13, ...,...
E    y: array([[[ 2.454723e-12,  1.490368e-12, -4.383434e-13, ...,
E            -1.402699e-12, -2.367054e-12, -4.383434e-13],
E           [-1.681645e-12, -8.432133e-12, -6.503422e-12, ...,...

I don't think it's related and I don't see it happening on master(82b3459). Maybe you can rebase @marsipu ?

GuillaumeFavelier

I tried it locally and it works as expected, I'm +1 for merge once CIs come back happy 👍

light	dark	custom™

drammock · 2021-03-19T17:24:11Z

This is what I saw:

I've seen that on another PR too, agree it's unrelated.

larsoner · 2021-03-19T17:29:51Z

Indeed on albertosottile/darkdetect#8 and this branch after installing qdarkstyle I get:

mne/viz/backends/_qt.py

hoechenberger · 2021-03-22T15:45:00Z

CIs are green – @marsipu @GuillaumeFavelier is this good to merge?

marsipu · 2021-03-22T15:50:50Z

We also talked about inverting the icon-pixmaps with the theme once and haven't done it yet. I am just trying to add it, but it may need another review from @GuillaumeFavelier.

larsoner · 2021-03-22T15:58:31Z

Yeah I think that's reasonable assuming it's not too difficult

…velier

…velier)

larsoner

Can you add a brain_kwargs=None to SourceEstimate.plot and VectorSourceEstimate.plot, and VolSourceEstimate.plot_3d? See for example how we handle add_data_kwargs, the difference being that brain_kwargs should be passed to the Brain constructor.

doc/changes/latest.inc

mne/viz/backends/_qt.py

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

marsipu · 2021-03-22T20:00:18Z

Can you add a brain_kwargs=None to SourceEstimate.plot and VectorSourceEstimate.plot, and VolSourceEstimate.plot_3d? See for example how we handle add_data_kwargs, the difference being that brain_kwargs should be passed to the Brain constructor.

I am sorry, I can't follow you: I already see the parameter brain_kwargs with default=None documented and established to pass kwargs on to the renderer (mne.viz._3d.py::1846 and mne.viz._3d.py::1857) for all mentioned plot-methods. Or do you mean something else?

larsoner · 2021-03-22T20:03:12Z

Oh I didn't realize we already had that param, never mind!

larsoner · 2021-03-22T20:03:30Z

Thanks @marsipu @GuillaumeFavelier !

agramfort · 2021-03-22T20:05:55Z

😎

hoechenberger · 2021-03-22T20:11:03Z

super good!

GuillaumeFavelier · 2021-03-23T08:24:00Z

Awesome work @marsipu 👌

GuillaumeFavelier mentioned this pull request Mar 18, 2021

WIP,ENH: Load Qt stylesheet #9126

Closed

1 task

larsoner reviewed Mar 18, 2021

View reviewed changes

mne/viz/_brain/_brain.py Outdated Show resolved Hide resolved

larsoner reviewed Mar 18, 2021

View reviewed changes

mne/viz/backends/_qt.py Outdated Show resolved Hide resolved

GuillaumeFavelier reviewed Mar 18, 2021

View reviewed changes

marsipu force-pushed the dark_brain branch from a353c04 to e012b3e Compare March 18, 2021 18:17

larsoner reviewed Mar 19, 2021

View reviewed changes

doc/conf.py Outdated Show resolved Hide resolved

mne/viz/_brain/_brain.py Outdated Show resolved Hide resolved

GuillaumeFavelier mentioned this pull request Mar 19, 2021

ENH: New interface for Brain #7162

Closed

86 tasks

marsipu force-pushed the dark_brain branch from 166faf2 to 2b889b8 Compare March 19, 2021 16:18

GuillaumeFavelier approved these changes Mar 19, 2021

View reviewed changes

larsoner reviewed Mar 19, 2021

View reviewed changes

mne/viz/backends/_qt.py Outdated Show resolved Hide resolved

hoechenberger approved these changes Mar 22, 2021

View reviewed changes

marsipu and others added 5 commits March 22, 2021 17:30

Add theme-parameter for dark-mode to PyVista-Brain (with @GuillaumeFa…

a02aadf

…velier)

Catching Import-Errors for optional packages

42886e0

Reorder

0a58c05

shorten documentation

6d96258

fix documentation cross-reference

f8f2d06

marsipu added 5 commits March 22, 2021 17:30

add to latest.inc

8fd1a1f

fix line [skip github][skip azp]

134e387

Make links work (thx to @larsoner)

b5b8926

Allow Error for faulty path as suggested by @larsoner[ci skip]

5c0da13

Inverting Colors of Icons (Version 1)

d4b7373

marsipu force-pushed the dark_brain branch from 711f999 to d4b7373 Compare March 22, 2021 16:32

marsipu added 2 commits March 22, 2021 17:41

Change Invert-Mode from Rgba to Rgb

6a393fd

Add newline [ci skip]

0ec587e

larsoner reviewed Mar 22, 2021

View reviewed changes

doc/changes/latest.inc Show resolved Hide resolved

mne/viz/backends/_qt.py Outdated Show resolved Hide resolved

Simplify _detect_theme

2a2a5d8

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Remove line [ci skip]

293a382

larsoner merged commit acf25ef into mne-tools:main Mar 22, 2021

marsipu deleted the dark_brain branch March 22, 2021 20:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dark-mode to PyVista-Brain (pair-programmed with @GuillaumeFavelier) #9149

Dark-mode to PyVista-Brain (pair-programmed with @GuillaumeFavelier) #9149

marsipu commented Mar 18, 2021 •

edited

Loading

larsoner Mar 18, 2021

marsipu Mar 18, 2021

GuillaumeFavelier Mar 18, 2021

GuillaumeFavelier commented Mar 18, 2021

marsipu commented Mar 18, 2021 •

edited

Loading

GuillaumeFavelier commented Mar 18, 2021

GuillaumeFavelier Mar 18, 2021

marsipu commented Mar 18, 2021 •

edited

Loading

larsoner commented Mar 18, 2021

larsoner commented Mar 18, 2021

marsipu commented Mar 19, 2021

GuillaumeFavelier commented Mar 19, 2021

GuillaumeFavelier left a comment

drammock commented Mar 19, 2021

larsoner commented Mar 19, 2021

hoechenberger commented Mar 22, 2021

marsipu commented Mar 22, 2021 •

edited

Loading

larsoner commented Mar 22, 2021

larsoner left a comment

marsipu commented Mar 22, 2021 •

edited

Loading

larsoner commented Mar 22, 2021

larsoner commented Mar 22, 2021

agramfort commented Mar 22, 2021

hoechenberger commented Mar 22, 2021

GuillaumeFavelier commented Mar 23, 2021

		to style the widgets. Automatic detection does not yet work on
		Linux. Can also be a path-like to a custom style sheet.

	`qdarkstyle` respectively `darkdetect` is required.
	:mod:`qdarkstyle` respectively :mod:`darkdetect` is required.

Dark-mode to PyVista-Brain (pair-programmed with @GuillaumeFavelier) #9149

Dark-mode to PyVista-Brain (pair-programmed with @GuillaumeFavelier) #9149

Conversation

marsipu commented Mar 18, 2021 • edited Loading

What does this implement/fix?

larsoner Mar 18, 2021

Choose a reason for hiding this comment

marsipu Mar 18, 2021

Choose a reason for hiding this comment

GuillaumeFavelier Mar 18, 2021

Choose a reason for hiding this comment

GuillaumeFavelier commented Mar 18, 2021

marsipu commented Mar 18, 2021 • edited Loading

GuillaumeFavelier commented Mar 18, 2021

GuillaumeFavelier Mar 18, 2021

Choose a reason for hiding this comment

marsipu commented Mar 18, 2021 • edited Loading

larsoner commented Mar 18, 2021

larsoner commented Mar 18, 2021

marsipu commented Mar 19, 2021

GuillaumeFavelier commented Mar 19, 2021

GuillaumeFavelier left a comment

Choose a reason for hiding this comment

drammock commented Mar 19, 2021

larsoner commented Mar 19, 2021

hoechenberger commented Mar 22, 2021

marsipu commented Mar 22, 2021 • edited Loading

larsoner commented Mar 22, 2021

larsoner left a comment

Choose a reason for hiding this comment

marsipu commented Mar 22, 2021 • edited Loading

larsoner commented Mar 22, 2021

larsoner commented Mar 22, 2021

agramfort commented Mar 22, 2021

hoechenberger commented Mar 22, 2021

GuillaumeFavelier commented Mar 23, 2021

marsipu commented Mar 18, 2021 •

edited

Loading

marsipu commented Mar 18, 2021 •

edited

Loading

marsipu commented Mar 18, 2021 •

edited

Loading

marsipu commented Mar 22, 2021 •

edited

Loading

marsipu commented Mar 22, 2021 •

edited

Loading