Return final output for generators in Client.predict #5057

freddyaboulton · 2023-08-01T15:22:10Z

Description

Closes #4887
Closes #4888

Adds a finish method to the client jobs to make it easier to wait for job completion.

Also installs the client locally for CI as discussed in #4888

Python

JS

🎯 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

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
You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

vercel · 2023-08-01T15:22:47Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
gradio	✅ Ready (Inspect)	Visit Preview	Aug 8, 2023 7:22pm

gradio-pr-bot · 2023-08-01T15:23:08Z

🎉 Chromatic build completed!

There are 0 visual changes to review.
There are 0 failed tests to fix.

gradio-pr-bot · 2023-08-01T15:23:36Z

🦄 change detected

This Pull Request includes changes to the following packages.

Package	Version
`@gradio/app`	`minor`
`gradio`	`minor`
`gradio_client`	`minor`

With the following changelog entry.

Client.predict will now return the final output for streaming endpoints

This is a breaking change (for gradio_client only)!

Previously, Client.predict would only return the first output of an endpoint that streamed results. This was causing confusion for developers that wanted to call these streaming demos via the client.

We realize that developers using the client don't know the internals of whether a demo streams or not, so we're changing the behavior of predict to match developer expectations.

Using Client.predict will now return the final output of a streaming endpoint. This will make it even easier to use gradio apps via the client.

⚠️ The changeset file for this pull request has been modified manually, so the changeset generation bot has been disabled. To go back into automatic mode, delete the changeset file.

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.

gradio-pr-bot · 2023-08-01T15:24:37Z

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

You can install the changes in this PR by running:

pip install https://gradio-builds.s3.amazonaws.com/35150c1a8a0aea0771cc97232634093ce19e102f/gradio-3.39.0-py3-none-any.whl

.github/workflows/backend.yml

abidlabs · 2023-08-02T16:59:55Z

client/js/src/client.ts

+					return { on, off, cancel, destroy, finish };
+				}
+
+				async function finish() {


abidlabs · 2023-08-02T17:22:29Z

Looks good @freddyaboulton! I tested the Python client and works great. Trying to figure out how to test the JS client right now.

A thought in the meantime -- I know we discussed this before and I was initially against it -- but should we just have the .result() method actually return the final result from .outputs() instead of the first result? 🙈

Pros:

People only need to remember one method and we don't need to have different code snippets
Its much simpler syntax than finish() and indexing the final result from outputs()
Its what people expect -- we've had a number of people asking why they are only getting the first response from generative outputs

Cons:

It would be a breaking change (but we are pre-release anyways)
We would have to handle every separately (but these endpoints are pretty rare anyways). Maybe we don't even expose endpoints that are every via API

freddyaboulton · 2023-08-02T17:53:36Z

I agree it's what people expect! Ok let's do that.

pngwn · 2023-08-08T17:18:08Z

client/js/src/client.ts

+					let result;

 					app
 						.on("data", (d) => {
-							data_returned = true;
 							if (status_complete) {
 								app.destroy();
+								data_returned = true;
+								res(d);
 							}
-							res(d);
+							result = d;
 						})
 						.on("status", (status) => {
 							if (status.stage === "error") rej(status);
-							if (status.stage === "complete" && data_returned) {
-								app.destroy();
-							}
 							if (status.stage === "complete") {
 								status_complete = true;
+								app.destroy();
+								res(result);


Whats this stuff?

Change to returning the final output of a generator.

Works in my tests but let me know if there’s a better way.

oh of course, my bad.

The only problem is it was implemented this way because i was having an issue receiving message out of order, which isn't technically supposed to be possible but it still happened. So the original logic allowed either message to resolve the promise whereas this only allows the status function to finish it, which if it is processed before the final data message means that the return value of predict would be the second to last value.

Maybe we could use the data.completed property and add the dual handling back?

Ok I added the dual handling back

pngwn

Looks good to me! Thanks @freddyaboulton!

freddyaboulton · 2023-08-09T17:03:53Z

Thank you @pngwn !!

freddyaboulton force-pushed the 4887-use-via-api-code-snippets branch from 0f12998 to 9846d4d Compare August 1, 2023 15:22

vercel bot deployed to Preview August 1, 2023 15:23 View deployment

vercel bot deployed to Preview August 1, 2023 15:24 View deployment

freddyaboulton commented Aug 1, 2023

View reviewed changes

.github/workflows/backend.yml Show resolved Hide resolved

freddyaboulton added the t: highlight A change that we wish to highlight label Aug 1, 2023

vercel bot deployed to Preview August 1, 2023 15:28 View deployment

vercel bot deployed to Preview August 1, 2023 15:36 View deployment

freddyaboulton force-pushed the 4887-use-via-api-code-snippets branch from 6653df7 to 5877ef0 Compare August 1, 2023 20:23

vercel bot deployed to Preview August 1, 2023 20:24 View deployment

freddyaboulton force-pushed the 4887-use-via-api-code-snippets branch from 5877ef0 to 1352f10 Compare August 1, 2023 20:25

vercel bot deployed to Preview August 1, 2023 20:26 View deployment

vercel bot deployed to Preview August 1, 2023 20:29 View deployment

freddyaboulton marked this pull request as ready for review August 1, 2023 20:38

freddyaboulton requested a review from pngwn August 1, 2023 20:40

vercel bot deployed to Preview August 1, 2023 20:42 View deployment

freddyaboulton requested review from abidlabs, aliabid94, aliabd, dawoodkhan82 and hannahblair August 1, 2023 20:50

vercel bot deployed to Preview August 1, 2023 20:50 View deployment

freddyaboulton force-pushed the 4887-use-via-api-code-snippets branch from caecc69 to 9b27061 Compare August 2, 2023 16:37

vercel bot deployed to Preview August 2, 2023 16:38 View deployment

abidlabs reviewed Aug 2, 2023

View reviewed changes

client/js/src/client.ts Outdated

return { on, off, cancel, destroy, finish };

}

async function finish() {

Copy link

Member

abidlabs Aug 2, 2023 •

edited

Choose a reason for hiding this comment

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

Nice js!

vercel bot deployed to Preview August 7, 2023 15:39 View deployment

freddyaboulton changed the title ~~Use submit in View API page for generators~~ Return final output for generators in Client.predict Aug 7, 2023

vercel bot deployed to Preview August 7, 2023 15:52 View deployment

vercel bot deployed to Preview August 7, 2023 16:28 View deployment

abidlabs mentioned this pull request Aug 8, 2023

Live audio streaming output #5077

Merged

pngwn reviewed Aug 8, 2023

View reviewed changes

freddyaboulton and others added 15 commits August 8, 2023 14:54

Add code

bf8f04a

add changeset

ec25886

add changeset

da7e746

Add feat changeset

7dcd64c

Fix js code snippet

55ea18e

Fix changelog

5848802

Add test

71a341a

Delete code

1ebbd3f

Lint

cabc65e

Make fix for python client

131bd70

Make JS client changes

e59f5a6

Add submit line to error

b010b84

Changelog

1718446

Lint

d6b2f5e

Undo

d1b9dbe

freddyaboulton force-pushed the 4887-use-via-api-code-snippets branch from 4f9b51b to d1b9dbe Compare August 8, 2023 18:54

vercel bot deployed to Preview August 8, 2023 18:56 View deployment

Add dual handling

186d376

vercel bot deployed to Preview August 8, 2023 19:22 View deployment

pngwn approved these changes Aug 9, 2023

View reviewed changes

freddyaboulton merged commit 35856f8 into main Aug 9, 2023
15 checks passed

freddyaboulton deleted the 4887-use-via-api-code-snippets branch August 9, 2023 17:03

pngwn mentioned this pull request Aug 9, 2023

chore: update versions #5038

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return final output for generators in Client.predict #5057

Return final output for generators in Client.predict #5057

freddyaboulton commented Aug 1, 2023 •

edited

vercel bot commented Aug 1, 2023 •

edited

gradio-pr-bot commented Aug 1, 2023 •

edited

gradio-pr-bot commented Aug 1, 2023 •

edited

Client.predict will now return the final output for streaming endpoints

This is a breaking change (for gradio_client only)!

Something isn't right?

gradio-pr-bot commented Aug 1, 2023 •

edited

abidlabs Aug 2, 2023 •

edited

abidlabs commented Aug 2, 2023

freddyaboulton commented Aug 2, 2023

pngwn Aug 8, 2023

freddyaboulton Aug 8, 2023

pngwn Aug 8, 2023

freddyaboulton Aug 8, 2023

pngwn left a comment •

edited

freddyaboulton commented Aug 9, 2023

Return final output for generators in Client.predict #5057

Return final output for generators in Client.predict #5057

Conversation

freddyaboulton commented Aug 1, 2023 • edited

Description

Python

JS

🎯 PRs Should Target Issues

Tests

vercel bot commented Aug 1, 2023 • edited

gradio-pr-bot commented Aug 1, 2023 • edited

gradio-pr-bot commented Aug 1, 2023 • edited

🦄 change detected

This Pull Request includes changes to the following packages.

With the following changelog entry.

Client.predict will now return the final output for streaming endpoints

This is a breaking change (for gradio_client only)!

Something isn't right?

gradio-pr-bot commented Aug 1, 2023 • edited

abidlabs Aug 2, 2023 • edited

Choose a reason for hiding this comment

abidlabs commented Aug 2, 2023

freddyaboulton commented Aug 2, 2023

pngwn Aug 8, 2023

Choose a reason for hiding this comment

freddyaboulton Aug 8, 2023

Choose a reason for hiding this comment

pngwn Aug 8, 2023

Choose a reason for hiding this comment

freddyaboulton Aug 8, 2023

Choose a reason for hiding this comment

pngwn left a comment • edited

Choose a reason for hiding this comment

freddyaboulton commented Aug 9, 2023

freddyaboulton commented Aug 1, 2023 •

edited

vercel bot commented Aug 1, 2023 •

edited

gradio-pr-bot commented Aug 1, 2023 •

edited

gradio-pr-bot commented Aug 1, 2023 •

edited

gradio-pr-bot commented Aug 1, 2023 •

edited

abidlabs Aug 2, 2023 •

edited

pngwn left a comment •

edited