Skip to content

Fix bugs with gr.update#2157

Merged
freddyaboulton merged 9 commits into
mainfrom
2154-fix-issues-with-update
Sep 6, 2022
Merged

Fix bugs with gr.update#2157
freddyaboulton merged 9 commits into
mainfrom
2154-fix-issues-with-update

Conversation

@freddyaboulton
Copy link
Copy Markdown
Collaborator

@freddyaboulton freddyaboulton commented Sep 1, 2022

Description

Fixes #2154 #2156

During #2044, I made it so that delete_none would keep None in the value key. This was to let users set the value of the component to null in the front-end which would reset the component value. The problem is that None is the default of the value key in the update component method, so the value is always reset, even when users are not trying to reset the value!!

The solution I went for was to distinguish the "don't update the value" and "set the value to null" use cases. For the former, we use an enum for the default value of the update method and for the latter, the user can return None as before.

The enum I chose is _Keywords.NO_VALUE. It's not exposed at the top-level gradio package and it's marked "private" by the leading underscore so it should not be autocompleted in IDES. So it should mean that the user did not pass in a value for the "value" key.

To test:

import gradio as gr

with gr.Blocks() as demo:
    radio = gr.Radio(label="algorithm", choices=["a", "b", "c"], value="a")
    slider = gr.Slider(label="threshold", value=0, minimum=-1, maximum=1, visible=False, interactive=True)
    style = gr.Dropdown(label="style", choices=["modern", "classical", "cartoon"], value="modern", visible=False, interactive=True)

    def make_visible(algorithm):
        is_visible = algorithm == "c"
        return {slider: gr.update(visible=is_visible), style: gr.update(visible=is_visible)}
    radio.change(make_visible, inputs=radio, outputs=[slider, style])

demo.launch()

The slider value and drop down should be persisted when clicking different radio buttons.

I also added blocks_update to all_demos - slider value should be persisted as well.

Checklist:

  • I have performed a self-review of my own code
  • 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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Sep 1, 2022

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

@freddyaboulton freddyaboulton marked this pull request as ready for review September 1, 2022 20:05
@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Sep 1, 2022

Just wanted to confirm a few things as I look into this PR:

  1. This would be a breaking change, right, since users can no longer use None to reset a component. But I guess it would not too breaking since this was just introduced in Reset components to original state by setting value to None #2044?

  2. Now to reset the value of a component, you pass in gr.VOID as the parameter to gr.update(value=)? an alternative might be gr.RESET if the purpose of the component is to reset a value.

  3. Finally, I wanted to ask whether you can also return gr.VOID from a function directly, instead of within a gr.update(value=). My preliminary experiments suggest that this doesn't work although I haven't tested thoroughly yet. I think we should have the same behavior if a user: (a) returns a value x from a function or (b) returns gr.update(value=x) from a function

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Sep 1, 2022

Also cc @aliabid94 for his feedback on this API

@freddyaboulton
Copy link
Copy Markdown
Collaborator Author

freddyaboulton commented Sep 1, 2022

@abidlabs Thanks for reviewing! Good questions!

  1. Yea I guess this is a breaking change but Reset components to original state by setting value to None #2044 was also technically a breaking change 🙈
  2. Yea I thought about gr.RESET but this doesn't reset to the original value, but rather it makes the value null in the front-end which was the goal of Make it easier to reset components to their original state when the app first launches #1672 . We can also do gr.NULL ?
  3. Right now the logic only works for updates. We can do it so you can return gr.VOID from a function but that would mean modifying all the component post process functions to replace VOID with None.

EDIT: Just thought of a corner case - what if a user wants to set the value of a text-box to the string "VOID". Need to use an enum so that there aren't conflicts

from enum import Enum

class Keywords(Enum):
    VOID = "VOID"

image

@williamberrios
Copy link
Copy Markdown

williamberrios commented Sep 5, 2022

Hi @freddyaboulton. It seems that when the output is only slider, the example doesn't work as expected


with gr.Blocks() as demo:
    radio = gr.Radio(label="algorithm", choices=["a", "b", "c"], value="a")
    slider = gr.Slider(label="threshold", value=0, minimum=-1, maximum=1, visible=False, interactive=True)

    def make_visible(algorithm):
        is_visible = algorithm == "c"
        return {slider: gr.update(visible=is_visible)}
    radio.change(make_visible, inputs=radio, outputs=[slider])

demo.launch(share = True)

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Sep 6, 2022

Hey @freddyaboulton sorry for the late comment, but could we use sentinels to distinguish between the user passing in None for the value parameter versus the user leaving the value parameter empty? Here's how numpy does it: https://stackoverflow.com/a/60157942/5209347

If this works, it has several advantages: (1) it's not a breaking change (2) it does not introduce any key words / additional syntax that a Gradio user needs to know

@freddyaboulton
Copy link
Copy Markdown
Collaborator Author

Hi @williamberrios ! Thanks for the find. That's a separate issue (not introduced in this PR). I filed an issue for that here: #2191

@freddyaboulton freddyaboulton force-pushed the 2154-fix-issues-with-update branch from 60db0bb to b566f37 Compare September 6, 2022 19:18
@freddyaboulton freddyaboulton changed the title Introduce a gr.VOID keyword to set component value to None Fix bugs with gr.update Sep 6, 2022
@freddyaboulton
Copy link
Copy Markdown
Collaborator Author

@abidlabs Thank you for the suggestion about "sentinel values". I think it does work - I implemented it by just setting the default value of value in the update methods to be the Keyword enum I introduced in this PR.

This should be good for another look!

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Sep 6, 2022

Will take a look now!

Comment thread gradio/components.py Outdated
Comment thread test/test_blocks.py Outdated
Comment thread test/test_blocks.py Outdated
@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Sep 6, 2022

Some minor comments, but LGTM! Nice fix

freddyaboulton and others added 2 commits September 6, 2022 16:58
Improve comment

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
Use NO_VALUE in tests

Co-authored-by: Abubakar Abid <abubakar@huggingface.co>
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.

3.2's gr.update(visible=X) causes value to be reset

3 participants