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

Quick fix: custom dropdown value #7567

Merged
merged 9 commits into from Mar 5, 2024
Merged

Quick fix: custom dropdown value #7567

merged 9 commits into from Mar 5, 2024

Conversation

dawoodkhan82
Copy link
Collaborator

@dawoodkhan82 dawoodkhan82 commented Feb 29, 2024

Description

Fixes:

import gradio as gr

text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value=None, allow_custom_value=True)
iface = gr.Interface(fn=lambda x: print(x), inputs=[text], outputs=[])
iface.launch()

Closes: #7549
Closes: #7613

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

  1. PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh

  2. You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

@dawoodkhan82 dawoodkhan82 changed the title Quick custom dropdown value fix Quick fix: custom dropdown value Feb 29, 2024
@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Feb 29, 2024

🪼 branch checks and previews

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

Install Gradio from this PR

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

Install Gradio Python Client from this PR

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

@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented Feb 29, 2024

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/dropdown patch
gradio patch
  • Maintainers can select this checkbox to manually select packages to update.

With the following changelog entry.

Quick fix: custom dropdown value

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.

@Paillat-dev
Copy link

By the way, the following is throwing the same error, including with this pr:

import gradio as gr

text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value=None, allow_custom_value=True)
iface = gr.Interface(fn=lambda x: [gr.Dropdown(choices=[], value=None)], inputs=[text], outputs=[text])
iface.launch()

@abidlabs
Copy link
Member

In addition to the issue @Paillat-dev pointed out, I noticed that the case of value="" is not handled correctly:

import gradio as gr

with gr.Blocks() as demo:
    text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value="", allow_custom_value=True, interactive=True)

demo.launch()

This should initialize the Dropdown with a value of "" but instead it initializes with "a" -- I think there could be a related issue happening.

Finally, could we add js unit tests for these fixes @dawoodkhan82?

@Paillat-dev
Copy link

There are some inconsistencies because if I use my second example above without lists as input and output values and return values but return directly the component it works perfectly fine.

@dawoodkhan82
Copy link
Collaborator Author

@Paillat-dev @abidlabs thanks for testing, taking a look at the issues.

@dawoodkhan82
Copy link
Collaborator Author

import gradio as gr

text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value=None, allow_custom_value=True)
iface = gr.Interface(fn=lambda x: gr.Dropdown(choices=[], value=None), inputs=[text], outputs=[text])
iface.launch()

@Paillat-dev This works fine for me by the way.

@abidlabs
Copy link
Member

abidlabs commented Mar 2, 2024

@dawoodkhan82 did you try submitting the Interface? That's when the error happened for me

@Paillat-dev
Copy link

Paillat-dev commented Mar 2, 2024

import gradio as gr

text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value=None, allow_custom_value=True)
iface = gr.Interface(fn=lambda x: gr.Dropdown(choices=[], value=None), inputs=[text], outputs=[text])
iface.launch()

@Paillat-dev This works fine for me by the way.

Might be a missunderstanding, but this indeed starts correctly, the error comes up when selecting something with the dropdown.

@dawoodkhan82
Copy link
Collaborator Author

In addition to the issue @Paillat-dev pointed out, I noticed that the case of value="" is not handled correctly:

Fixed

@dawoodkhan82
Copy link
Collaborator Author

@abidlabs are you testing this code?

import gradio as gr

text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value=None, allow_custom_value=True)
iface = gr.Interface(fn=lambda x: gr.Dropdown(choices=[], value=None), inputs=[text], outputs=[text])
iface.launch()
Screen.Recording.2024-03-04.at.9.43.45.AM.mov

Works for me, when not returning the dropdown as a list. I'm not sure returning a list is supposed to work

@Paillat-dev
Copy link

To me it feels weird, since it works when returning multiple items as a list, just not with one. And especially when in the function the outputs are still specified as a list.

@abidlabs
Copy link
Member

abidlabs commented Mar 4, 2024

Ah good point @dawoodkhan82!

@Paillat-dev when you have a single output component, you should not wrap the return in a list / tuple. You should just return the output as is. The reason we don't support singleton lists is that it becomes impossible to distinguish whether you are intending on returning a singleton list or the item within it in some scenarios. Consider this example:

import gradio as gr

gr.Interface(lambda x:[{"abc": "def"}], "textbox", "json").launch()

In this case, both are possible: the user might be intending on returning a list, or just the dict within it. When you have multiple output components, this ambiguity is resolved.

So in that case, this PR looks good to me @dawoodkhan82 if we could just add a js unit test to prevent regressions!

@Paillat-dev
Copy link

I did not think of that. Sounds logical now, thanks for the explanation. And thanks for the fix :)

@dawoodkhan82
Copy link
Collaborator Author

In addition to the issue @Paillat-dev pointed out, I noticed that the case of value="" is not handled correctly:

import gradio as gr

with gr.Blocks() as demo:
    text = gr.Dropdown(["a", "b", "c"], label="Select a letter", value="", allow_custom_value=True, interactive=True)

demo.launch()

This should initialize the Dropdown with a value of "" but instead it initializes with "a" -- I think there could be a related issue happening.

Finally, could we add js unit tests for these fixes @dawoodkhan82?

Taking a look at this again, and according to the dropdown tests. When initializing the dropdown with value="", it should pre-select the first dropdown option.

@abidlabs
Copy link
Member

abidlabs commented Mar 5, 2024

Taking a look at this again, and according to the dropdown tests. When initializing the dropdown with value="", it should pre-select the first dropdown option.

Oh weird, is that the case when initializing with any value that is not one of the choices? I guess that makes sense

@abidlabs
Copy link
Member

abidlabs commented Mar 5, 2024

LGTM @dawoodkhan82! Thanks for adding the test.

I've confirmed that this fixes: #7613 so I've added it to the parent comment.

I'll go ahead and merge this in so that we can get it out for the release!

@abidlabs abidlabs merged commit e340894 into main Mar 5, 2024
7 of 8 checks passed
@abidlabs abidlabs deleted the dropdown-fix branch March 5, 2024 23:21
@pngwn pngwn mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants