Skip to content

Load Event Trigger#2456

Merged
dawoodkhan82 merged 17 commits into
mainfrom
dawood/upload-event
Oct 26, 2022
Merged

Load Event Trigger#2456
dawoodkhan82 merged 17 commits into
mainfrom
dawood/upload-event

Conversation

@dawoodkhan82
Copy link
Copy Markdown
Collaborator

Description

Add event trigger for when a user uploads to video, audio, file, and image components.

Screen.Recording.2022-10-13.at.5.16.16.PM.mov
import gradio as gr

def test(video):
    return video

with gr.Blocks() as demo:
    video1 = gr.Video()
    video2 = gr.Video()
    video1.load(test, video1, video2)

demo.launch()

Please include:

  • relevant motivation
  • a summary of the change
  • which issue is fixed.
  • any additional dependencies that are required for this change.

Closes: #2419

Checklist:

  • I have performed a self-review of my own code
  • I have added a short summary of my change to the CHANGELOG.md
  • My code follows the style guidelines of this project
  • I have commented my code in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

A note about the CHANGELOG

Hello 👋 and thank you for contributing to Gradio!

All pull requests must update the change log located in CHANGELOG.md, unless the pull request is labeled with the "no-changelog-update" label.

Please add a brief summary of the change to the Upcoming Release > Full Changelog section of the CHANGELOG.md file and include
a link to the PR (formatted in markdown) and a link to your github profile (if you like). For example, "* Added a cool new feature by [@myusername](link-to-your-github-profile) in [PR 11111](https://github.com/gradio-app/gradio/pull/11111)".

If you would like to elaborate on your change further, feel free to include a longer explanation in the other sections.
If you would like an image/gif/video showcasing your feature, it may be best to edit the CHANGELOG file using the
GitHub web UI since that lets you upload files directly via drag-and-drop.

Comment thread CHANGELOG.md
* Fix bug where new typeable slider doesn't respect the minimum and maximum values [@dawoodkhan82](https://github.com/dawoodkhan82) in [PR 2380](https://github.com/gradio-app/gradio/pull/2380)
* Add guide on creating a map demo using the `gr.Plot()` component [@dawoodkhan82](https://github.com/dawoodkhan82) in [PR 2402](https://github.com/gradio-app/gradio/pull/2402)
* Add blur event for `Textbox` and `Number` components [@dawoodkhan82](https://github.com/dawoodkhan82) in [PR 2448](https://github.com/gradio-app/gradio/pull/2448)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Moving these to the right section

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@freddyaboulton Seems like changelog check is failing. Is it because I'm moving these to the right section?

@github-actions
Copy link
Copy Markdown
Contributor

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2456-all-demos

@abidlabs
Copy link
Copy Markdown
Member

Very cool @dawoodkhan82! Tested and works great for all four of the components. I do have one suggestion, which is to rename the event from .load() to .upload() for two reasons:

  • We already use .load() as an event trigger for Blocks() where it has a different meaning: it triggers the function call when the Blocks page loads. Whereas here we are using it differently, so this might be confusing to users.

  • This new event trigger has no effect when the source="webcam" or "canvas", or anything besides source="upload" (tested and confirmed). So calling it .upload() will be consistent with this fact

I also think that every component should have a version of this event trigger. For example, Textbox() should have a .type() trigger which only fires when someone types in the textbox, but not if the change happens due to a function outputting into the textbox. This would allow us to close #1431 for example. But we can save this for a later PR

@pngwn
Copy link
Copy Markdown
Member

pngwn commented Oct 14, 2022

I think this is a good addition until we change the API but I'm not sure this really fixes #2419 because change will still be firing every time the value changes (which it should), in order to separate the two events some pretty unreliable user code is required.

An app author would need to track when both events are triggering in order to distinguish them, but I don't think that is reliable unless we send a session ID with each request and I'm not sure if we document the order in which events are fired or maintain that as a part of the API, so the required user code to make this work would be even more complex.

So this fixes #2419 in one direction but not in the other.

Comment thread CHANGELOG.md Outdated
Comment thread gradio/events.py
Copy link
Copy Markdown
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 minor comments, but otherwise looks good!

@dawoodkhan82 dawoodkhan82 merged commit 83a038e into main Oct 26, 2022
@dawoodkhan82 dawoodkhan82 deleted the dawood/upload-event branch October 26, 2022 00:58
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.

Identify the source of the change event (automatic, or manual upload) for video/audio components

3 participants