-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[MRG+1] Draggable colorbar. #3290
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.
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
Conversation
mne/time_frequency/tfr.py
Outdated
| if 'grad' in self: | ||
| types.append('grad') | ||
| chs = [ch for ch in self.ch_names if ch.startswith('MEG') and | ||
| ch.endswith(('2', '3'))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is neuromag specificity. Why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to #3063
I'll fix it.
|
Thanks @jaeilepp for taking the time to do this ! From what I understand this would work for divergent colormaps. Are you planning on addressing the sequential cases (e.g. combined gradiometers, potentially time frequency spectrum)? In matplotlib they typically use one mouse button for displacement and another for scaling, so we could have a similar approach for clim WDYT? |
|
Now the color map can be changed with up/down keys. Clicking and dragging changes the scale. Left mouse button moves it up and down and right mouse button changes the lims. |
Current coverage is 86.67%@@ master #3290 diff @@
==========================================
Files 335 335
Lines 57322 57625 +303
Methods 0 0
Messages 0 0
Branches 8715 8780 +65
==========================================
+ Hits 49567 49947 +380
+ Misses 5132 5003 -129
- Partials 2623 2675 +52
|
mne/viz/topo.py
Outdated
| cmap = 'RdBu_r' | ||
| interactive_cmap = True | ||
| else: | ||
| interactive_cmap = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
interactive_cmap = False
if cmap == 'interactive':
cmap = 'RdBu_r'
interactive_cmap = True
|
LGTM |
|
Now interactive mode works for tfr, images and topomaps. For topos it is probably too heavy. Ready for review. |
|
Can you rebase? Then I'll try it and merge if it's all good |
doc/whats_new.rst
Outdated
|
|
||
| - Added the ability to decimate :class:`mne.Evoked` objects with :func:`mne.Evoked.decimate` by `Eric Larson`_ | ||
|
|
||
| - Add 'cmap=interactive' option to plotting functions by `Jaakko Leppakangas`_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
double-backticks on the quote will read nicer, and interactive should probably be a string in that code bit
|
Can you mention quickly the examples you've been running to test this to save reviewers some |
I've had my own scripts for testing, but it works for tfr, topomap and image. So |
|
Does |
I think we should probably remove the option if it isn't really usable. At the very least we should add a |
|
Works nicely in |
Yeah, it's quite slow. I'll investigate.
It's not enabled for topographies |
Yeah, why not. |
Sorry, I meant for |
|
Now cmap is accepted as tuple, where the second element defines the interacitivity. It is also on by default except with topomaps when there are more than two of them. |
|
LGTM and the interaction is nice in e.g. If someone else has you make changes, it might be nice to add |
|
Very nice! Indeed. +1 for showing this in one example or even a gif to be tweeted cc @kingjr |
That's basically the default behavior now, but I'll add it somewhere since I'm adding a test anyway. |
|
@jaeilepp can you just update the viz tutorial(s) to document this? then +1 for merge |
|
you can add a section of changing the colormap eg in the epochs viz tutorial |
|
Ready for review/merge. |
|
Thanks @jaeilepp |
Closes #3005
Also:
closes #3063