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

Easier method to programmatically change the current param value #183

Open
ianhi opened this issue Apr 27, 2021 · 2 comments · May be fixed by #189
Open

Easier method to programmatically change the current param value #183

ianhi opened this issue Apr 27, 2021 · 2 comments · May be fixed by #189
Labels
enhancement New feature or request

Comments

@ianhi
Copy link
Collaborator

ianhi commented Apr 27, 2021

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:

  1. Uniform user facing API for matplotlib and ipywidgets sliders
  2. Don't have to replicate every method, just the important ones
  3. Is perhaps a reasonable way to leave the door open to other slider libs in the future
  4. Less need to explain the nightmare of HBoxes for ipywidgets sliders

cons:

  1. Have to replicate two different APIs
  2. users will not get direct access to the slider
    • well they could with some explanation
  3. using type on this may be a confusing experience
    • relatedly, what if it gets passed back into a controls object

I 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:

  1. Easiest to implement

cons:

  1. Doesn't really give full functionality (what about setting the slider label)
  2. What about non-slider widgets?

Easy access to the underlying sliders
pros:

  1. pretty easy to implement

cons:

  1. Do we give back the whole hbox for ipywidgets sliders?

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 HBoxs 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 some

@ianhi ianhi added the enhancement New feature or request label Apr 27, 2021
@ianhi
Copy link
Collaborator Author

ianhi commented Apr 27, 2021

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 value and set_val but I'm wondering about eliminating the set_val - although that may provide a more reasonable way to deal with autogenerated sliders that use underlying intsliders.

@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

@redeboer
Copy link
Contributor

redeboer commented Apr 27, 2021

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 AbstractSlider class is not really abstract btw. What it's trying to do is encapsulate an mpl widget or ipywidget by mimicking a union of their properties. It depends a bit on the purpose of mpl-interactions whether this approach is a good idea. If the main goal is to make it easier to create interactive matplotlib plots, my gut feeling is that it would be better to stick with one interface only (making it truly abstract) and propagate that behaviour through mpl-interactions.

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 Slider/Controller first.

Abstract controller/slider interface snippet
from 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

    ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants