-
Notifications
You must be signed in to change notification settings - Fork 8
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
Live ipywidget functionality #93
Conversation
@christian-oreilly I reworked the example to use a dict instead of an array for each widget to make it more readable. No changes to functionality. Let me know if you have any thoughts on file structure and if anything should be rearranged. Otherwise, this should be good to merge. |
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.
Let's take the habit of clearing out the output of notebook cells to clean up our commit diffs. You can clear them manually in the notebook, but you can also run a command like this:
jupyter nbconvert --clear-output --inplace <your notebook>.ipynb
This could be automated through DevOps. I think it is a good idea. I'll open a separate issue for that.
Could you please clear the notebooks output, commit, and push? Meanwhile I'll continue looking at that code a bit.
What about defining the class like this instead? class WidgetObserver:
""" Sets an observed for `widget` on the `attribute` of `object`. """
def __init__(self, widget, object, attribute):
self.widget = widget
self.object = object
self.attribute = attribute
self._observe()
def _widget_change(self, change):
setattr(self.image, self.attribute, change["new"])
def _observe(self):
self.widget.observe(self._widget_change, names=["value"]) It automates a bit of the boilerplate. The example would become simply import ipywidgets as wgt
from ipyniivue import NiiVue, SliceType, WidgetObserver
volumes = [
{ "path": "../images/mni152.nii.gz", "colormap": "gray",
"visible": True, "opacity": 1.0 },
{ "path": "../images/hippo.nii.gz", "colormap": "red",
"visible": True, "opacity": 1.0 },
]
nv = NiiVue(slice_type=SliceType.MULTIPLANAR)
nv.load_volumes(volumes)
# Slice type
widget_slice_type = wgt.RadioButtons(options=[('Axial', 0),
('Coronal', 1),
('Sagittal', 2),
('Multiplanar', 3),
('Render', 4)],
value=3,
description='Slice Type:')
WidgetObserver(widget=widget_slice_type, object=nv, attribute="slice_type")
# Scan opacity
widget_scan_opacity = wgt.FloatSlider(value=1.0, min=0.0, max=1.0,
step=0.1, description='Scan Opacity:',
orientation='horizontal')
WidgetObserver(widget=widget_scan_opacity, object=nv.volumes[0],
attribute="opacity")
# Hippocampus opacity
widget_hippo_opacity = wgt.FloatSlider(value=1.0, min=0.0,
max=1.0, step=0.1,
description='Hippocampus Opacity:',
orientation='horizontal')
WidgetObserver(widget=widget_hippo_opacity, object=nv.volumes[1],
attribute="opacity")
# Scan colormap
widget_scan_colormap = wgt.Select(options=['Gray', 'Red', 'Blue', 'Green'],
value='Gray', description='Scan Colormap:')
WidgetObserver(widget=widget_scan_colormap, object=nv.volumes[0],
attribute="colormap")
# Hippocampus colormap
widget_hippo_colormap = wgt.Select(options=['Red', 'Blue', 'Green', 'Gray'],
value='Red', description='Hippocampus Colormap:')
WidgetObserver(widget=widget_hippo_colormap, object=nv.volumes[1],
attribute="colormap") This code was not tested! |
@christian-oreilly I changed the WidgetChange class to your WidgetObserver class with a small change to the variable names to get it working (it was still using the "image" from my code and not the updated "object"). Additionally, I reworked it in the example to use dictionaries and for loops instead of the direct call of WidgetObserver that you had for each widget. I think in the long run, if you were to have many widgets for one NiiVue instance, this would be clearer and more readable this way. However, if you think otherwise, let me know as it would be easy to switch back. Additionally, I believe the notebook cells should all be cleared. I will take a look at the issue you opened when I have the chance. |
Sure. At this level, there is a bit of "esthetical preferences", so I am fine with either. However, in the current form, you could simplify by replacing: for widget in widgetArray:
WidgetObserver(widget=widget["widget"], object=widget["object"],
attribute=widget["attribute"]) by for widget in widgetArray:
WidgetObserver(**widget) I'll leave it up to you to decide which version you prefer to use there. |
Good point. I simplified the WidgetObserver call with your recommendation. Good to merge. |
I created a new class WidgetChange() in ._widget that can be used in an ipywidgets observe to bring live updates from any widgets to the NiiVue rendering. Additionally, I created a new example "widgets.ipynb" to show off this live widget functionality.
Currently the WidgetChange class is just put at the bottom of the widget.py file where it doesn't really fit in. There is no better place for it right now, but down the line it may be better to separate more function-like classes from the other classes (i.e. NiiVue, Volume, Mesh) which operate more like an object.
Additionally, I added the Mac specific file .DS_Store to .gitignore. Small change not worthy of its own PR.
I'm going to rework the way widgets are set up in the example file to make it more readable, so for now, I will leave this as a draft.