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 Timer component #8505

Merged
merged 54 commits into from
Jun 28, 2024
Merged

Add Timer component #8505

merged 54 commits into from
Jun 28, 2024

Conversation

aliabid94
Copy link
Collaborator

@aliabid94 aliabid94 commented Jun 7, 2024

  1. Implemented gr.Timer, a Component meant to replace every, e.g. timer = gr.Timer(5) for a 5 second regular timer. Can be used via:
  • a tick event listener, e.g. timer.tick(fn=..., inputs=[], outputs=[])
  • an every= that can accept a timer `gr.Textbox(value=lambda: ..., every=timer)
  1. Also added inputs= kwarg to make function values more useful, e.g. gr.Textbox(value=lambda a, b: ..., inputs=[text1, text2]). The component will automatically re-update if any of the the input components change.

Can all be combined for maximum "dashboard" shorthand: gr.Textbox(value=lambda a, b: ..., inputs=[text1, text2], every=timer), which sets the value of the component as a function of the inputs, updating every timer tick and also when the inputs change.

See demo file that uses all these concepts:

import gradio as gr
import random

countries = [
    "Algeria", "Argentina", "Australia", "Brazil", "Canada", "China", "Democratic Republic of the Congo", "Greenland (Denmark)", "India", "Kazakhstan", "Mexico", "Mongolia", "Peru", "Russia", "Saudi Arabia", "Sudan", "United States"
]

with gr.Blocks() as demo:
    with gr.Row():
        count = gr.Slider(1, 10, step=1, label="Country Count")
        alpha_order = gr.Checkbox(True, label="Alphabetical Order")

    gr.JSON(lambda count, alpha_order: countries[:count] if alpha_order else countries[-count:], inputs=[count, alpha_order])
    timer = gr.Timer(1)
    with gr.Row():
        gr.Textbox(lambda: random.choice(countries), label="Random Country", every=timer)
        gr.Textbox(lambda count: ", ".join(random.sample(countries, count)), inputs=count, label="Random Countries", every=timer)
    with gr.Row():
        gr.Button("Start").click(lambda: gr.Timer(active=True), None, timer)
        gr.Button("Stop").click(lambda: gr.Timer(active=False), None, timer)


if __name__ == "__main__":
    demo.launch()

Fixes: #8582
Fixes: #6577
Fixes: #8463
Fixes: #7051
Fixes: #6841

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jun 7, 2024

🪼 branch checks and previews

Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
Storybook ready! Storybook preview
🦄 Changes detected! Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/de843f3a33d01c19e810a15ee4263fd6b7929449/gradio-4.37.1-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@de843f3a33d01c19e810a15ee4263fd6b7929449#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-builds.s3.amazonaws.com/de843f3a33d01c19e810a15ee4263fd6b7929449/gradio-client-1.2.0.tgz

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Jun 7, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/app minor
@gradio/client minor
@gradio/timer minor
gradio minor
gradio_client minor
website minor
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Add Timer component

Maintainers or the PR author can modify the PR title to modify this entry.

Something isn't right?

  • Maintainers can change the version label to modify the version bump.
  • If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can update the changelog file directly.

@abidlabs
Copy link
Member

abidlabs commented Jun 11, 2024

Hmm I'm finding this syntax unnecessarily complicated in this PR and internal message:

with gr.Blocks() as demo:
  def show_plot_1():
    ...
  def show_plot_2():
    ...

  with gr.Timer(5) as timer:
    plot_1 = gr.LinePlot(value=show_plot_1)
    plot_2 = gr.LinePlot(value=show_plot_2)
  • I don't think it makes sense for gr.Timer() to be a context manager. First of all, this introduces a completely new use of context managers in Gradio, as right now, we only use context managers to set the layouts. Secondly, this fuses two things which should remain separate: the position of components and whether they are controlled by the same timer. Imagine that you have a row of line plots, but only some of them (say every other one) should be running every 5 seconds. This context manager syntax would be very cumbersome to apply

That being said, I do see the value in having a separate gr.Timer() object, which can be started/stopped/adjusted. I would instead make the every argument also be able to take a gr.Timer() instance, which can be started/stopped/adjusted as your PR implements.

So rewriting your example code:

with gr.Blocks() as demo:
  def show_plot_1():
    ...
  def show_plot_2():
    ...

  timer = gr.Timer(5)
  plot_1 = gr.LinePlot(value=show_plot_1, every=timer)
  plot_2 = gr.LinePlot(value=show_plot_2, every=timer)

@abidlabs
Copy link
Member

Also added inputs= kwarg to make function values more useful, e.g. gr.Textbox(value=lambda a, b: ..., inputs=[text1, text2])

This addition to the API does feel useful to me ✔️

@freddyaboulton
Copy link
Collaborator

I would instead make the every argument also be able to take a gr.Timer() instance, which can be started/stopped/adjusted as your PR implements.

I like this suggestion.

Also, I think right now the timer only triggers tick event and value functions. Can we make it trigger any event on a schedule?

For a use case where you want to start the schedule based on a button click or tab select, you have to do something like this

with gr.Blocks() as demo:
    timer = gr.Timer(5, active=False)
    plot = gr.LinePlot(value=get_data, every=timer)
    button = gr.Button("Start")
    button.click(lambda: gr.Timer(active=True), None, timer)

If we let it trigger any event, the code could be rewritten like the following which feels more natural:

with gr.Blocks() as demo:
    plot = gr.LinePlot()
    button = gr.Button("Start")
    button.click(get_data, None, plot, every=gr.Timer(5))

@aliabid94
Copy link
Collaborator Author

don't think it makes sense for gr.Timer() to be a context manager... instead make the every argument also be able to take a gr.Timer() instance

Okay sure. Originally my logic for using context managers for Timer and InputSet had been that in a dashboard, every component is tied to the same timer and set of inputs, and its annoying to have to set the timer and the inputs for each component, but that's something we can design some other time.

I think right now the timer only triggers tick event and value functions. Can we make it trigger any event on a schedule? ...e.g. button.click(get_data, None, plot, every=gr.Timer(5))

A little confused by this syntax. Does the syntax above start the given timer, and then bind this same event to the tick of the Timer? If it gets clicked again, it does nothing?

Without any button control, the syntax is very clear:

plot = gr.Plot()
timer = gr.Timer(5)
timer.tick(get_data, None, plot)

or simply:

plot = gr.Plot(get_data, every = gr.Timer(5))

If you are adding a button to start the timer, then you probably want to be able to stop it too, so you need access to the timer object. In that case, you are suggesting:

plot = gr.Plot()
timer = gr.Timer(5)
button.click(get_data, None, plot, every=timer)
button2.click(lambda: gr.Timer(active=False), outputs=timer)

correct? Imo that's messy compared to:

timer = gr.Timer(5)
plot = gr.Plot(get_data, every=timer)
button.click(lambda: gr.Timer(active=True), outputs=timer)
button2.click(lambda: gr.Timer(active=False), outputs=timer)

The second option is much more explicit, takes the same amount of code, and starting and stopping use the same syntax.

I don't think it makes sense to add every to event listeners. It's a decent shorthand for components that have function values though.

@freddyaboulton
Copy link
Collaborator

Ok let's stick with the original proposal to have every only apply to component value functions.

@aliabid94 aliabid94 marked this pull request as ready for review June 13, 2024 22:28
@aliabid94
Copy link
Collaborator Author

I believe everything has been addressed, could use a re-review cc @abidlabs, also @pngwn if you can look at my approach to avoid calls stacking up when the browser is minimized - this was happening because window.dispatchEvent was triggering while minimized, but window.addEventListener("gradio") was not issuing callbacks until the window was restored. Fixed by only firing ticks if the document is visible

@@ -453,11 +453,7 @@ def predict(
client.predict(5, "add", 4, api_name="/predict")
>> 9.0
"""
inferred_fn_index = self._infer_fn_index(api_name, fn_index)
if self.endpoints[inferred_fn_index].is_continuous:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we still need this when the Client connects to Spaces running older versions of Gradio?

@@ -398,7 +427,7 @@ def event_trigger(
preprocess: If False, will not run preprocessing of component data before running 'fn' (e.g. leaving it as a base64 string if this method is called with the `Image` component).
postprocess: If False, will not run postprocessing of component data before returning 'fn' output to the browser.
cancels: A list of other events to cancel when this listener is triggered. For example, setting cancels=[click_event] will cancel the click_event, where click_event is the return value of another components .click method. Functions that have not yet run (or generators that are iterating) will be cancelled, but functions that are currently running will be allowed to finish.
every: Run this event 'every' number of seconds while the client connection is open. Interpreted in seconds.
every: Will be deprecated in favor of gr.Timer. Run this event 'every' number of seconds while the client connection is open. Interpreted in seconds.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does every for events behave differently than in components? We should just have the same behavior here, where you can provide a timer or a float?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to encourage the use of every added to an event listener which is a very weird API imo, so I'm only supporting floats for the sake of backwards compatibility.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not? Its quite an intuitive api -- once the event is triggered, it should rerun every X seconds. Its much easier than creating a gr.Timer object and setting it to active, etc.

Copy link
Collaborator Author

@aliabid94 aliabid94 Jun 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the 5th (!!!) way to support a continuous event, we already have:

  1. using a direct timer with a tick listener
  2. using every=float in a Component, which creates a timer under the hood
  3. using every=Timer in a Component with an existing explicit timer
  4. setting every=float in an event listener, to specifically say, this event also start a timer that binds to this same event (I would like to deprecate this one)

We keep coming up way too many ways to do the same thing and it makes the API messier and messier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But these are not the same thing. Using every in a Component only allows you to rerun a function when the demo first loads. Whereas using every in an event listener allows you to rerun a function when an event is triggered.

In fact, I would argue that this would make things more consistent. Just like every in a Component can be either a float or a Timer, every in an event listener can be a float or a Timer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Synced with @aliabid94 and I'm convinced that its not a good idea to add yet another approach.

Copy link
Member

@abidlabs abidlabs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few nits, but can otherwise confirm that this works great. Great stuff @aliabid94!

@abidlabs
Copy link
Member

all good from my end!

@aliabid94 aliabid94 merged commit 2943d6d into main Jun 28, 2024
8 checks passed
@aliabid94 aliabid94 deleted the timer_event_listener branch June 28, 2024 23:53
@pngwn pngwn mentioned this pull request Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v: minor A change that requires a minor release
Projects
None yet
6 participants