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.

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

MRG: Prototype of notebook viz (ipyvtk) #8503

Merged

Conversation

GuillaumeFavelier
Copy link
Contributor

@GuillaumeFavelier GuillaumeFavelier commented Nov 10, 2020

This PR updates the current notebook 3d backend to use ipyvtk. It allows native support for widgets, 3d render view and intuitive camera interactions. It could also support google colab and jupyter lab? More test is needed.

It's still a work in progress:

output

code
import os
import mne
from mne.datasets import sample
mne.viz.set_3d_backend('notebook')
data_path = sample.data_path()
sample_dir = os.path.join(data_path, 'MEG', 'sample')
subjects_dir = os.path.join(data_path, 'subjects')
fname_stc = os.path.join(sample_dir, 'sample_audvis-meg')
stc = mne.read_source_estimate(fname_stc, subject='sample')
initial_time = 0.13
brain = stc.plot(subjects_dir=subjects_dir, initial_time=initial_time,
                 clim=dict(kind='value', pos_lims=[3, 6, 9]),
                 time_viewer=True,
                 show_traces=True,
                 hemi='split',
                 size=600,
                 background="white",
                 )

@GuillaumeFavelier GuillaumeFavelier self-assigned this Nov 10, 2020
@hoechenberger
Copy link
Member

😍😍😍

@GuillaumeFavelier
Copy link
Contributor Author

Update after 4d9b63a:

output

I think we could use ipywidgets to reproduce the tool bar buttons.

I started by working directly on Brain but next I'll see how the other 3d plots behave.

@GuillaumeFavelier
Copy link
Contributor Author

Update after dcaff44:

output

@larsoner
Copy link
Member

Lots of - lines is nice, and it seems to work well! I assume this is server-side rendering piping PNGs to the client?

Anything else left to do here?

@christianhacker is this more like what you want to use?

@larsoner
Copy link
Member

Anything else left to do here?

I guess one thing from seeing what you've shown so far would be making sure that show_traces=True adds a matplotlib plot below. Would this require a lot of hacking of the backend to get right? Maybe this is a PR we can merge early in the 0.23 cycle so people can play with it.

@GuillaumeFavelier
Copy link
Contributor Author

I assume this is server-side rendering piping PNGs to the client?

After digging into https://github.com/Kitware/ipyvtk-simple/blob/master/ipyvtk_simple/viewer.py#L163-L171 I can confirm this indeed. JPEG though :)

Anything else left to do here?

  • shortcuts
  • toolbar
  • matplotlib canvas

Would this require a lot of hacking of the backend to get right?

I don't think so...

@GuillaumeFavelier
Copy link
Contributor Author

Update after ec0d265:

output

self.axes = self.fig.add_subplot(111)
self.notebook = notebook
if self.notebook:
self.fig, self.axes = plt.subplots()
Copy link
Member

Choose a reason for hiding this comment

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

if you set the figsize and dpi and such it will even be the right size! Not 100% sure how you get it to show up below the brain plot, probably has to do with when each is instantiated which seems like it could be a pain to manage

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know where to get correct values for dpi. We were using Qt for that before

Copy link
Member

Choose a reason for hiding this comment

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

I would try just setting it to 100. I don't know if it will matter. If it does we can figure out the right thing to do. One reason you need it is because there figsize is in inches, so you need to know the DPI to get from the pixel width to the figsize.

@christianhacker
Copy link

It's beautiful...

I'll test it out when I have more time, but this looks very good. Thank you for your hard work.

@larsoner
Copy link
Member

For shortcuts I'm not sure where to look. I'd see how matplotlib handles it.

For toolbar I'd also look at how matplotlib makes theirs and use the same code

@GuillaumeFavelier
Copy link
Contributor Author

Update after c254d79:

The matplotlib figure events are now connected correctly.

Supported shortcuts:

  • i toggle slider visibility
  • s apply auto scaling
  • r restore scaling
  • c clear glyphs

Not supported (yet):

  • play (this one was based on QThread, I need a workaround)
  • ? help (just does not work for me)
  • ctrl+shift+s save movie (based on modifiers, I need to find a solution too)

output

@agramfort
Copy link
Member

please update what's new page and then +1 for MRG when CIs are green

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

+1 to merge when green !

@agramfort
Copy link
Member

ok to merge for you @larsoner ?

mne/viz/_brain/_brain.py Outdated Show resolved Hide resolved
mne/viz/_brain/_brain.py Outdated Show resolved Hide resolved
Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

LGTM +1 for merge once CIs are happy

@GuillaumeFavelier GuillaumeFavelier merged commit cab1425 into mne-tools:master Jan 6, 2021
@GuillaumeFavelier GuillaumeFavelier deleted the proto/ipyvtk_backend branch January 6, 2021 16:24
@agramfort
Copy link
Member

🍻 🍾 🎉 !!!

@GuillaumeFavelier can you tweet a screenshot movie of this?

@GuillaumeFavelier
Copy link
Contributor Author

Unfortunately, I'm not on Twitter 😅

I can still prepare a video though 👍

@GuillaumeFavelier
Copy link
Contributor Author

Here is a simple MP4 demo :

output.mp4

@drammock
Copy link
Member

drammock commented Jan 6, 2021

Unfortunately, I'm not on Twitter sweat_smile

I think you mean fortunately I'm not on twitter :)

@agramfort
Copy link
Member

agramfort commented Jan 6, 2021 via email

larsoner added a commit to larsoner/mne-python that referenced this pull request Jan 7, 2021
* upstream/master:
  MAINT: Actually use all caches (mne-tools#8702)
  MRG: Prototype of notebook viz (ipyvtk) (mne-tools#8503)
  MRG: Use caching in Github Actions and Azure Pipelines (mne-tools#8695)
cbrnr pushed a commit to cbrnr/mne-python that referenced this pull request Jan 15, 2021
* Deploy basic version

* Update prototype [skip ci]

* Update _Renderer [skip ci]

* Update picking [skip ci]

* Fix disp

* Add shortcuts [skip ci]

* Update tests

* Add ipyvtk_simple

* Fix style

* Update tests

* Refactor shortcuts

* Improve tests

* Remove cruft

* Remove cruft

* Reorder figures

* Add tool bar

* Reorder figures

* Fix visibility

* Improve coverage

* Fix ratio and layout

* Update changes

* Do not import pyplot

* Update mne/viz/_brain/_brain.py

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

* Use _add_action

* Use icon + tooltip

* Fix qt func

* Reorder actions

* Switch to icon + description

Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants