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

ObjectSelector/DiscreteSlider should handle empty/length-one object lists gracefully #361

Closed
shenker opened this issue Apr 2, 2019 · 6 comments

Comments

@shenker
Copy link

shenker commented Apr 2, 2019

param.ObjectSelector turns into a DiscreteSlider instead of a Select when displayed as a widget only if it is initialized with objects containing numerical values. In particular, this leads to two unexpected behaviors:

  1. ObjectSelector cannot be initialized with objects lists containing zero or one numeric value (they can be initialized with zero or one non-numeric value).
  2. If an ObjectSelector is initialized with all numeric values, this produces a different widget than what you see if its initialized with non-numeric values.

Motivation: In cases where the list of options is known ahead of time and fixed at runtime, this is fine. But I'm using ObjectSelectors to filter on columns of a datasource; this means that whether I get a DiscreteSlider or a Select is determined by the properties of my data in an unexpected way. Worse, my app may error out or not (because of an empty list or singleton list of objects) depending on this initial condition.

Possible resolutions to the empty/singleton objects issue:

  1. At the very least, this behavior should be documented.
  2. Bokeh slider could be fixed to allow zero or one elements
  3. Panel could work around the bokeh slider bug (maybe by adding two dummy values to the slider and disabling the widget?)

Possible resolutions to the indeterminacy issue:

  1. Add a separate NumericSelector type to param (which is displayed as a slider) and make param.ObjectSelector always show a Select widget.

Thoughts?

Example:

import holoviews as hv
import param
import panel as pn
hv.extension('bokeh')

class Test(param.Parameterized):
    foo = param.ObjectSelector()
    
    def __init__(self, objects):
        self.param.foo.objects = objects
        super().__init__()

pn.Param(Test(['a','b'])) # works
pn.Param(Test([1,2])) # works
pn.Param(Test([])) # works
pn.Param(Test([1])) # ValueError: Slider 'start' and 'end' cannot be equal.

Full traceback:

~/projects/panel/panel/widgets/base.py in _get_model(self, doc, root, parent, comm)
     42 
     43     def _get_model(self, doc, root=None, parent=None, comm=None):
---> 44         model = self._widget_type(**self._process_param_change(self._init_properties()))
     45         if root is None:
     46             root = model

~/.local/share/virtualenvs/molecule-counting-YM9FlNJN/lib/python3.7/site-packages/bokeh/models/widgets/sliders.py in __init__(self, **kwargs)
     59         if 'start' in kwargs and 'end' in kwargs:
     60             if kwargs['start'] == kwargs['end']:
---> 61                 raise ValueError("Slider 'start' and 'end' cannot be equal.")
     62         super(Widget, self).__init__(**kwargs)
     63 

ValueError: Slider 'start' and 'end' cannot be equal.
@jbednar
Copy link
Member

jbednar commented Apr 3, 2019

It sounds like Panel should handle the case containing zero or one numeric value, for which we can simply show a static text widget, with no slider needed (@philippjfr?). As for avoiding switching widget types, you can simply specify the widget type at the Panel level for that parameter, overriding the default logic, but I don't recall the syntax for that; I think it's somewhere in the docs, though.

@xavArtley
Copy link
Collaborator

xavArtley commented Apr 3, 2019

The syntax to choose the type of widgets is the following for this example:

pn.Param(Test([1]),widgets={'foo':pn.widgets.Select})

produce :
image

@philippjfr
Copy link
Member

It sounds like Panel should handle the case containing zero or one numeric value, for which we can simply show a static text widget, with no slider needed

Yes, that seems like the appropriate behavior, although I wish sliders at least didn't error when start and end are the same.

@philippjfr
Copy link
Member

I also think defaulting to Select may be more appropriate.

@philippjfr
Copy link
Member

If you want a DiscreteSlider you'll still be able to be explicit about it, but I think by default making the fewest assumptions is best.

@jbednar
Copy link
Member

jbednar commented Apr 3, 2019

Ok, sounds good. A little less magic...

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

No branches or pull requests

4 participants