gradio-app / gradio

Build and share delightful machine learning apps, all in Python. 🌟 Star to support our work!
http://www.gradio.app
Apache License 2.0
29.49k stars 2.19k forks source link

Add support for keyword args in JS client #8187

Closed hannahblair closed 2 weeks ago

hannahblair commented 2 weeks ago

Description

This PR adds support for passing keyword arguments as well as positional args in the JS client. This is backwards compatible, so the two usages will work:

const result = await app.predict({
    endpoint: "/predict",
    data: ["Chewbacca"]
});
const result = await app.predict("/predict", ["Chewbacca"]);

Closes: #7831

🎯 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

gradio-pr-bot commented 2 weeks ago

🪼 branch checks and previews

• Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
Storybook ready! Storybook preview
:unicorn: Changes detected! Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/8433ab389c178dfb4a8b39f859054fd81b3c0bcc/gradio-4.28.3-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@8433ab389c178dfb4a8b39f859054fd81b3c0bcc#subdirectory=client/python"
gradio-pr-bot commented 2 weeks ago

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
@gradio/app minor
@gradio/client minor
gradio minor

With the following changelog entry.

Add support for keyword args in JS client

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](https://github.com/gradio-app/gradio/edit/jsclient-kwargs/.changeset/red-colts-burn.md).
pngwn commented 2 weeks ago

I don't think was the intention with that issue. I think it is keyword arguments specifically for the data. The endpoint isn't optional in the JS client so this would be something like:

const result = await app.predict("/predict", {
  audio: audio_file,
  textbox: "Chewbacca"
});

What i don't understand is what those key correspond to. @abidlabs could you clarify what the keys respond and how we figure out what they are?

abidlabs commented 2 weeks ago

Thanks @hannahblair @pngwn yes the idea of #7831 is to allow users to provide the data as a dictionary where keys are parameter names and the values are the data. The benefit of this is (a) it would make it clearer which argument is being passed to which parameter, which can help prevent errors, especially with many parameters or similar-looking parameters (b) allow us to utilize default values of parameters i.e. make some keys optional if a developer provides a default value in the function

What i don't understand is what those key correspond to. @abidlabs could you clarify what the keys respond and how we figure out what they are?

The keys can be obtained using the "/info" route. If you look at the "/info" route, each endpoint has a "parameters" key whose dictionary now looks like this:

"parameters": [
  {
    "label": "x",
    "parameter_name": "x",
    "parameter_has_default": false,
    "parameter_default": null,
    "type": {
    "type": "string"
  },
}
...
],

The key should be value of parameter_name, whether or not it has a default is indicated by parameter_has_default (so if this is true, the key should be considered optional), and the value of that default is indicated by parameter_default

abidlabs commented 2 weeks ago

By the way, this is the corresponding function in the Python Client which handles the logic of constructing the positional arguments from the keyword arguments:

https://github.com/gradio-app/gradio/blob/9a869d93590d3120bb1b6957ac7524700e07f9ed/client/python/gradio_client/utils.py#L1095

We should also make sure that the documentation is updated to reflect the new keyword-based usage, I think there are three places:

(1) in the Guide for the JS Client (2) in the View API page, we'll need to update the JS Client code snippet (3) in the .view_api() method of the Client

pngwn commented 2 weeks ago

The only issue with is that we won't have any types, although we might be able to make it generic and provide a snippet. Maybe as part of #5462.

hannahblair commented 2 weeks ago

@abidlabs @pngwn thanks for correcting + explaining! closing this PR and branch in favour of #8197 which is ready for review

and to your point @pngwn, agreed that it'd be nice to address the lack of types in #5462