-
Notifications
You must be signed in to change notification settings - Fork 20
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
Easier method to programmatically change the current param value #183
Comments
Turns out I'm a bit partial to the AbstractSlider idea, and I think we can put off doing that for other widget types. Here's a rough implementation. The only maptlotlib vs ipywidgets induced duplication is @redeboer is this the sort of object oriented api you were envisioning (#181 (comment))? rough AbstractSlider implementation
from matplotlib.widgets import Slider as mpl_slider
class AbstractSlider:
"""
A slider that abstracts over matplotlib and ipywidgets sliders
You can always accesss the underlying slider through the `.raw_slider` attribute.
"""
def __init__(slider, arr=None):
self.slider = slider
self.mpl_slider = isinstance(slider, mpl_slider)
@property
def value(self):
if self.mpl_slider:
v = self.slider.val
else:
v = self.slider.value
if arr is not None:
return arr[v]
else:
return v
@value.setter
def value(self, new_value):
# add some sort of check for `arr` here?
# i.e. if this is really an intslider that we're using as a floatslider
if self.mpl_slider:
self.slider.set_val(new_value)
else:
self.slider.value = new_value
@property
def min(self):
if self.mpl_slider:
return self.slider.valmin
else:
return self.slider.min
@min.setter
def min(self, value):
if self.mpl_slider:
self.slider.valmin = value
else:
self.slider.min = value
@property
def max(self):
if self.mpl_slider:
return self.slider.valmax
else:
return self.slider.max
@max.setter
def max(self, value):
if self.mpl_slider:
self.slider.valmax = value
else:
self.slider.max = value
@property
def step(self):
if self.mpl_slider:
return self.slider.valstep
else:
return self.slider.step
@step.setter
def step(self, value):
if self.mpl_slider:
self.slider.valstep = value
else:
self.slider.step = value
@property
def description(self):
if self.mpl_slider:
return self.slider.label.get_text()
else:
return self.slider.description
@description.setter
def description(self, value):
if self.mpl_slider:
self.slider.label.set_text(value)
else:
self.slider.description = value
# matplotlib style methods
def set_val(self, new_value):
"""
Set the value of the slider. This equivalent to directly assigning to the `.value` attribute.
This function provides the matplotlib slider style way rather than the ipywidgets way.
"""
self.value = new_value
# this is a case where I think the maptlotlib api is more reasonable than the
# ipywidgets api
def on_change(self, func):
"""
Parameters
----------
func : Callable
With signature `func(new_value)`
"""
if self.mpl_slider:
self.slider.on_changed(func)
else:
def f(change):
return func(change['new'])
self.slider.observe(f, names='value')
def set_label(self):
# the same as description - this is mpl's terminology
pass |
Great sketch of the situation! A general comment (also with regard to #183 (comment)): it was through #182 and the related issue (particularly #181 (comment)) that I started to understand that mpl-interactions tries to bridge ipywidgets and matplotlib widgets. (This is not immediately clear from the documentation, but it would be a laudable aim.) As such, I think the abstract class idea you sketched is best. The The snippet below illustrates such an abstract interface. It may be a bit bothersome (this is not even sketching implementation classes), but the major advantage of defining (abstract) interfaces is that they signal and enforce the expected behaviour for the rest of the framework. The consequence would be that you have to stick with one interface and cannot directly plug in and duck type sliders from external libraries (ipywidgets and mpl) -- they'd have to be converted to some implementation of Abstract controller/slider interface snippetfrom abc import ABC, abstractmethod
class Controller(ABC):
# Could kick out this interface -- only makes sense if all controllers (like a
# selector) have some property in common
@abstractmethod
def on_change(self, func):
pass
@abstractmethod
@property
def label(self) -> str:
pass
@abstractmethod
@label.setter
def label(self, new_label: str) -> None:
pass
class Slider(Controller):
@abstractmethod
@property
def value(self):
pass
@abstractmethod
@value.setter
def value(self, new_value):
pass
@abstractmethod
@property
def min(self):
pass
@abstractmethod
@min.setter
def min(self, value):
pass
... |
Problem
If you want to directly modify the current value of a slider generated by this package it is very difficult. Currently it is undocumented, and confusingly/difficult would be a kind way of putting it. For example see the discussion in #181 about using
controls.controls['tau'].children[0]
.There needs to be an easier (and documented!!) way to change the attributes of the sliders.
Proposed Solutions
Create an abstract Slider class
Create a Slider like object that abstracts over matplotlib and ipywidgets sliders. It could have it's own set of documented methods or take on the important methods of both maptlotlib sliders and ipywidgets sliders
pros:
HBox
es for ipywidgets sliderscons:
type
on this may be a confusing experienceI think the last point really only matters if you pass in an explicitly created slider, and then expect to get it back. Though hopefully this isn't too common.
Methods for setting on the controls object
e.g.
controls.set_param_value('tau', 5)
pros:
cons:
Easy access to the underlying sliders
pros:
cons:
Looking at this there might be argument for doing all three. Or perhaps just the first two.
If we don't go with the first option then there will need to be a docs page explaining the nonsense with
HBox
s for ipywidgets sldiers.Additional context
I have no idea how if theres any reasonable way to have a depcreation path here, I suppose it would require keeping the
.controls
attribute with a warning on access and moving to someThe text was updated successfully, but these errors were encountered: