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

Fix of value_throttled in editable sliders #2208

Merged
merged 6 commits into from
Apr 20, 2021
Merged

Fix of value_throttled in editable sliders #2208

merged 6 commits into from
Apr 20, 2021

Conversation

hoxbro
Copy link
Member

@hoxbro hoxbro commented Apr 18, 2021

Fixes #2204

The error in #2204 was related to a missing edit_constant and the possibility of multiple events in _sync_value, which should now be fixed for the three editable sliders.

When looking / testing the editable sliders I could not get value_throttled to work with the _input_widget. To fix this, I changed the communication from jscallback to param.watch. Furthermore, I don't completely understand why in the current design it is possible to exceed the limit of the bounds with the _input_widget like

Current This PR
before after

I would expect the bounds to set to newer be exceeded, but it seems to be possible by design. Therefore, I have only implemented it for EditableIntSlider and EditableFloatSlider.

@codecov
Copy link

codecov bot commented Apr 18, 2021

Codecov Report

❗ No coverage uploaded for pull request base (master@4533a60). Click here to learn what that means.
The diff coverage is 10.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #2208   +/-   ##
=========================================
  Coverage          ?   83.77%           
=========================================
  Files             ?      182           
  Lines             ?    21802           
  Branches          ?        0           
=========================================
  Hits              ?    18265           
  Misses            ?     3537           
  Partials          ?        0           
Impacted Files Coverage Δ
panel/widgets/slider.py 75.50% <10.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4533a60...9c414c7. Read the comment docs.

@philippjfr
Copy link
Member

Furthermore, I don't completely understand why in the current design it is possible to exceed the limit of the bounds with the _input_widget like

Very much by design indeed, there's many scenarios where this is useful. That said there's an argument to be made that this behavior should be togglable and/or it should restore the original bounds if you exceed the bounds and then restore them. @jbednar Any thoughts on that?

@hoxbro
Copy link
Member Author

hoxbro commented Apr 18, 2021

Added the functionality to be togglable:

after2

@jbednar
Copy link
Member

jbednar commented Apr 18, 2021

Thanks, but a single "exceedable" switch is not sufficent, because a widget needs to accept both exceedable and non-exceedable (hard and soft) bounds on both the lower and upper sides. A hard bound is an absolute limit -- it is an error to set the value outside those bounds. A soft bound or suggested range helps set the slider up to be useful, but can totally be exceeded if needed. Think about it as the soft bounds being for the user, helping the user have a slider that covers a useful range, and the hard bounds are for the program, making sure that the value never exceeds an allowed range so that the program doesn't crash (e.g. by avoiding negative numbers where they are not allowed).

So here, we don't want an "exceed bounds" flag, because (a) that would apply to both the high and low which many have different characteristics, and (b) just because we want people to be able to go outside the soft bounds doesn't mean we want them to be able to go outside all conceivable bounds; there are still often hard bounds to respect.

E.g. many physical quantities like length or mass or area never allow negative numbers and allow arbitrarily large values in theory, but for a given app there's some finite range that's typical (e.g. 10 to 100). So we'd set a hard bounds of 0,None and a soft bounds of 10,100. The slider would go between 10 and 100, while the spinner would allow any positive number. Without the "100" we couldn't get a slider (as we can't have a slider ranging up to infinity), but it's arbitrary; we just have to pick something, and then let the user override it. Without the 0 we'll have to have assertions or exception handling in our code to catch negative numbers. Only with both full hard and soft bound support can we handle everything.

See bokeh/bokeh#9943 for the full discussion.

@hoxbro
Copy link
Member Author

hoxbro commented Apr 18, 2021

I was not aware of the discussion behind this. I only looked at the development documentation, where the feature of exceeding start and end is not mentioned. So thank you for the detailed explanation.

I will remove the exceed_bounds asap and focus this PR on getting value_throttled working for the editable sliders.

@hoxbro
Copy link
Member Author

hoxbro commented Apr 18, 2021

Value throttled should now work for the editable sliders.

Code
import panel as pn


edit_float_slider = pn.widgets.EditableFloatSlider(name="Editable FloatSlider")


@pn.depends(edit_float_slider.param.value_throttled, watch=True)
def test_throttled_10(v1):
  print(v1)


edit_int_slider = pn.widgets.EditableIntSlider(name="Editable IntSlider", start=0, end=8, step=2, value=4)


@pn.depends(edit_int_slider.param.value_throttled, watch=True)
def test_throttled_11(v1):
  print(v1)


edit_range_slider = pn.widgets.EditableRangeSlider(name="Editable RangeSlider", start=0, end=8, step=2, value=(1,4))


@pn.depends(edit_range_slider.param.value_throttled, watch=True)
def test_throttled_12(v1):
  print(v1)


pn.Column(edit_float_slider, edit_int_slider, edit_range_slider,).servable()

after2

Some notes / comments :

  1. If I don't add value to _slider_widget, value_throttled do not work for edit_int_slider when going to zero:
  • with value in slider_widget
    with
  • without value in slider_widget
    without
  1. EditableFloatSlider´s FloatInput properly need a format like EditableRangeSlider.
  2. The EditableRangeSlider´s _end_edit should always be higher than _start_edit and the other way around
    limit

@philippjfr philippjfr merged commit 7c2fe60 into holoviz:master Apr 20, 2021
@hoxbro hoxbro deleted the throttled_edit branch May 6, 2021 09:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants