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

Add a throttled argument to depends, interact and bind #1754

Merged
merged 8 commits into from Nov 17, 2020
Merged

Add a throttled argument to depends, interact and bind #1754

merged 8 commits into from Nov 17, 2020

Conversation

Hoxbro
Copy link
Member

@Hoxbro Hoxbro commented Nov 9, 2020

Supersede #1400

The goal of this PR is to discuss the option to add a argument to pn.depends , pn.interact and pn.bind which throttles the slider widgets as mention in #1259.

This is a work in progress and await input from @philippjfr.

For testing purpose I use this script:

from datetime import datetime, date

import panel as pn

pn.extension()

slider_dict = {
    "DateSlider": dict(start=date(2018, 9, 1), end=date(2018, 9, 10)),
    "DateRangeSlider": dict(
        start=date(2018, 9, 1),
        end=date(2018, 9, 10),
        value=(date(2018, 9, 2), date(2018, 9, 4)),
    ),
    "DiscreteSlider": dict(
        options=[0.1, 1, 10, 100],
        value=1,
    ),
    "FloatSlider": dict(start=1, end=10, value=5),
    "IntSlider": dict(start=1, end=10, value=5),
    "IntRangeSlider": dict(start=1, end=10, value=(2, 5)),
    "RangeSlider": dict(start=1, end=10, value=(2, 5)),
}

func = lambda x: repr(x)
throttled = True

dashboard = []
for slider, kwargs in slider_dict.items():
    widget = getattr(pn.widgets, slider)(**kwargs)
    bind = pn.bind(func, widget, throttled=throttled)
    depends = pn.depends(widget, throttled=throttled)(func)
    interact = pn.interact(func, x=widget, throttled=throttled)
    slider = pn.Column("## " + slider, widget, bind, depends, interact)
    dashboard.append(slider)

pn.Row(*dashboard).show()

Missing tasks:

  • Discussion if this is the best implementation or the implementation is needed.
  • Keyword argument to be used to enable value_throttled on sliders, right now it is throttled.
  • Create tests for the chosen implementation.
  • Update documentation with the new implementation.
  • Other?

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #1754 (36b7c6e) into master (06530a0) will increase coverage by 0.19%.
The diff coverage is 92.68%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1754      +/-   ##
==========================================
+ Coverage   84.93%   85.12%   +0.19%     
==========================================
  Files         151      151              
  Lines       17420    17941     +521     
==========================================
+ Hits        14796    15273     +477     
- Misses       2624     2668      +44     
Impacted Files Coverage Δ
panel/interact.py 74.57% <71.42%> (+0.63%) ⬆️
panel/widgets/slider.py 93.95% <81.81%> (+2.25%) ⬆️
panel/tests/test_interact.py 99.00% <83.33%> (-1.00%) ⬇️
panel/layout/card.py 95.74% <100.00%> (ø)
panel/tests/test_server.py 100.00% <100.00%> (ø)
panel/tests/widgets/test_slider.py 100.00% <100.00%> (ø)
panel/viewable.py 71.54% <0.00%> (-1.77%) ⬇️
panel/pane/holoviews.py 84.04% <0.00%> (-1.03%) ⬇️
panel/command/serve.py 16.84% <0.00%> (-0.55%) ⬇️
panel/compiler.py 11.11% <0.00%> (-0.32%) ⬇️
... and 79 more

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 556c1f6...36b7c6e. Read the comment docs.

@philippjfr
Copy link
Member

I'm a -1 on the changes to bind and depends since you can very easily use widget.param.value_throttled.

@philippjfr
Copy link
Member

So what I'd like to see here is removing it from the depends/bind code, adding an example to the interact docs and finally a test.

@philippjfr
Copy link
Member

Also happy to see an example showing depends/bind usage with value_throttled, don't have any suggestions on where that should go though.

@Hoxbro
Copy link
Member Author

Hoxbro commented Nov 10, 2020

Thanks for the comments, I will do what you have suggested later this week.

I would just like to mention that I have had a lot of problem finding out the correct way to apply value_throttled argument in depends/interact, when I started learning Panel and I still find myself look at old code to get it to work. I can see I'm properly not the only one which have these problem if I look at the discourse 1416 and 1404, so maybe a documentation page with only documentation and examples of how to use value_throttled could be a good option. This page could then be referenced at each of the slider widgets page.

@MarcSkovMadsen
Copy link
Collaborator

@Hoxbro. Why do you use value_throtled at all and what does it do? (I've never used that.)

@xavArtley
Copy link
Collaborator

xavArtley commented Nov 13, 2020 via email

@Hoxbro
Copy link
Member Author

Hoxbro commented Nov 13, 2020

As @xavArtley explained it is for calling a function after the mouse is released from the slider and not continually doing the sliding. When the function takes a long time to calculate it becomes very frustrating because it feels like it takes even longer than it should, see example below. Also if the function has to open a file or look up values from a database it will do it a unnecessary amount of times.

import panel as pn
import time

pn.extension()

def func(x):
    time.sleep(1)
    return repr(x)

widget1 = pn.widgets.IntSlider(start=1, end=100)
widget2 = pn.widgets.IntSlider(start=1, end=100)

slider1 = pn.bind(func, widget1)
slider2 = pn.bind(func, widget2.param.value_throttled)

pn.Column(widget1, slider1, widget2, slider2).servable()

ezgif-4-e1a154739680

Right now the only options is to add the widget.param.value_throttled as an input instead of just widget. When you have a lot of sliders it becomes more confusing and therefore it would have been nice if there were an option to just use value_throttled instead of value if a widget had it.

Another problem with value_throttled (to my knowledge) is that it is hard to incorporate into a parametrized class, what I do is either what I showed here or just create a "simple" widget.

@philippjfr
Copy link
Member

Thanks so much! I'll merge this for now, and further improvements for the docs can come later.

@philippjfr
Copy link
Member

The Parameterized class problem is a good point, I think we should allow configuring that in the Param pane widgets argument. Also for a later PR though.

@philippjfr philippjfr merged commit 5e43280 into holoviz:master Nov 17, 2020
@Hoxbro Hoxbro deleted the throttled_argument branch November 17, 2020 17:07
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

Successfully merging this pull request may close these issues.

None yet

4 participants