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

ENH: Add a playback button to the notebook 3d backend #8741

Merged
merged 17 commits into from
May 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions mne/viz/_brain/_brain.py
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,14 @@ def _configure_dock(self):
self._renderer._dock_finalize()

def _configure_playback(self):
self._renderer._playback_initialize(self._play, self.refresh_rate_ms)
self._renderer._playback_initialize(
func=self._play,
timeout=self.refresh_rate_ms,
value=self._data['time_idx'],
rng=[0, len(self._data['time']) - 1],
time_widget=self.widgets["time"],
play_widget=self.widgets["play"],
)

def _configure_mplcanvas(self):
# Get the fractional components for the brain and mpl
Expand Down Expand Up @@ -1243,11 +1250,11 @@ def _configure_tool_bar(self):
)
self._renderer._tool_bar_add_button(
name="visibility",
desc="Toggle Visibility",
desc="Toggle Controls",
func=self.toggle_interface,
icon_name="visibility_on"
)
self._renderer._tool_bar_add_button(
self.widgets["play"] = self._renderer._tool_bar_add_play_button(
name="play",
desc="Play/Pause",
func=self.toggle_playback,
Expand Down Expand Up @@ -1345,7 +1352,14 @@ def _on_button_release(self, vtk_picker, event):
if self._mouse_no_mvt > 0:
x, y = vtk_picker.GetEventPosition()
# programmatically detect the picked renderer
self.picked_renderer = self.plotter.iren.FindPokedRenderer(x, y)
try:
# pyvista<0.30.0
self.picked_renderer = \
self.plotter.iren.FindPokedRenderer(x, y)
except AttributeError:
# pyvista>=0.30.0
self.picked_renderer = \
self.plotter.iren.interactor.FindPokedRenderer(x, y)
Comment on lines +1355 to +1362
Copy link
Member

Choose a reason for hiding this comment

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

Like this block works for >= 0.30 and < 0.30

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But this one does not depend on another package.

If the user does not have ipyvtklink but has pyvista 0.30.0, it just won't work.

Copy link
Contributor Author

@GuillaumeFavelier GuillaumeFavelier May 3, 2021

Choose a reason for hiding this comment

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

That's why I think bumping is the least painful solution

# trigger the pick
self.plotter.picker.Pick(x, y, 0, self.picked_renderer)
self._mouse_no_mvt = 0
Expand Down
4 changes: 3 additions & 1 deletion mne/viz/_brain/tests/test_notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,9 @@ def interactive(on):
brain._renderer.actions['screenshot_field'].value = screenshot_path
total_number_of_buttons = sum(
'_field' not in k for k in brain._renderer.actions.keys())
number_of_buttons = 0
assert 'play' in brain._renderer.actions
# play is not a button widget, it does not have a click() method
number_of_buttons = 1
for action in brain._renderer.actions.values():
if isinstance(action, Button):
action.click()
Expand Down
7 changes: 6 additions & 1 deletion mne/viz/backends/_abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,10 @@ def _tool_bar_add_spacer(self):
def _tool_bar_add_file_button(self, name, desc, func, shortcut=None):
pass

@abstractmethod
def _tool_bar_add_play_button(self, name, desc, func, shortcut=None):
pass

@abstractmethod
def _tool_bar_set_theme(self, theme):
pass
Expand Down Expand Up @@ -573,7 +577,8 @@ def _status_bar_update(self):

class _AbstractPlayback(ABC):
@abstractmethod
def _playback_initialize(self, func, timeout):
def _playback_initialize(self, func, timeout, value, rng,
time_widget, play_widget):
pass


Expand Down
33 changes: 24 additions & 9 deletions mne/viz/backends/_notebook.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
# License: Simplified BSD

from contextlib import contextmanager, nullcontext
from distutils.version import LooseVersion

import pyvista
from IPython.display import display
from ipywidgets import (Button, Dropdown, FloatSlider, FloatText, HBox,
IntSlider, IntText, Text, VBox, IntProgress)
IntSlider, IntText, Text, VBox, IntProgress, Play,
jsdlink)

from ._abstract import (_AbstractDock, _AbstractToolBar, _AbstractMenuBar,
_AbstractStatusBar, _AbstractLayout, _AbstractWidget,
Expand All @@ -23,7 +26,8 @@ def _layout_initialize(self, max_width):

def _layout_add_widget(self, layout, widget, stretch=0):
widget.layout.margin = "2px 0px 2px 0px"
widget.layout.min_width = "0px"
if not isinstance(widget, Play):
widget.layout.min_width = "0px"
children = list(layout.children)
children.append(widget)
layout.children = tuple(children)
Expand Down Expand Up @@ -189,6 +193,12 @@ def callback():
func=callback,
)

def _tool_bar_add_play_button(self, name, desc, func, shortcut=None):
widget = Play(interval=500)
self._layout_add_widget(self._tool_bar_layout, widget)
self.actions[name] = widget
return _IpyWidget(widget)

def _tool_bar_set_theme(self, theme):
pass

Expand Down Expand Up @@ -224,8 +234,15 @@ def _status_bar_update(self):


class _IpyPlayback(_AbstractPlayback):
def _playback_initialize(self, func, timeout):
pass
def _playback_initialize(self, func, timeout, value, rng,
time_widget, play_widget):
play = play_widget._widget
play.min = rng[0]
play.max = rng[1]
play.value = value
slider = time_widget._widget
jsdlink((play, 'value'), (slider, 'value'))
jsdlink((slider, 'value'), (play, 'value'))


class _IpyMplInterface(_AbstractMplInterface):
Expand Down Expand Up @@ -341,14 +358,12 @@ def show(self):
self._create_default_tool_bar()
display(self._tool_bar)
# viewer
try:
# pyvista<0.30.0
if LooseVersion(pyvista.__version__) < LooseVersion('0.30'):
viewer = self.plotter.show(
use_ipyvtk=True, return_viewer=True)
except RuntimeError:
# pyvista>=0.30.0
else: # pyvista>=0.30.0
viewer = self.plotter.show(
jupyter_backend="ipyvtk_simple", return_viewer=True)
jupyter_backend="ipyvtklink", return_viewer=True)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a try/except as well?

Copy link
Contributor Author

@GuillaumeFavelier GuillaumeFavelier May 3, 2021

Choose a reason for hiding this comment

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

Since 0.30.0 pyvista does not use ipyvtk_simple anymore

Copy link
Member

Choose a reason for hiding this comment

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

Yes but I'm thinking that it should also work for people with 0.29 if possible

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 plan to bump/update those requirements in #9341

Copy link
Member

Choose a reason for hiding this comment

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

If it's only a line or two to keep it working, it would be nice not to force people to upgrade

Copy link
Member

Choose a reason for hiding this comment

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

But if they have ipyvtk_simple and pyvista < 0.30 it should work, right? For example:

kwargs = dict()
if LooseVersion(pyvista.__version__) < LooseVersion('0.30'):
    kwargs['jupyter_backend' ] = 'ipyvtk_simple'
else:
    kwargs['jupyter_backend'] = 'ipyvtklink'
viewer = self.plotter.show(return_viewer=True, **kwargs)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would be nice not to force people to upgrade

I understand

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'll rework this part to use LooseVersion instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The API changed too, the following won't work:

self.plotter.show(jupyter_backend="ipyvtk_simple", return_viewer=True)

viewer.layout.width = None # unlock the fixed layout
# main widget
if self._dock is None:
Expand Down
6 changes: 5 additions & 1 deletion mne/viz/backends/_qt.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,9 @@ def callback():
shortcut=shortcut,
)

def _tool_bar_add_play_button(self, name, desc, func, shortcut=None):
self._tool_bar_add_button(name, desc, func, None, shortcut)

def _tool_bar_set_theme(self, theme):
if theme == 'auto':
theme = _detect_theme()
Expand Down Expand Up @@ -324,7 +327,8 @@ def _status_bar_update(self):


class _QtPlayback(_AbstractPlayback):
def _playback_initialize(self, func, timeout):
def _playback_initialize(self, func, timeout, value, rng,
time_widget, play_widget):
self.figure.plotter.add_callback(func, timeout)


Expand Down