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
34.13k stars 2.6k forks source link

better typing for `@gradio/client` #5462

Open pngwn opened 1 year ago

pngwn commented 1 year ago

Is your feature request related to a problem? Please describe.

gradioApp.predict() has return type Promise. It could be something better than unknown (maybe use generics like gradioApp.predict() will return Promise<GradioRes>

Will add more details soon

Describe the solution you'd like
A clear and concise description of what you want to happen.

Additional context
Add any other context or screenshots about the feature request here.

Ifeanyi55 commented 1 year ago

I agree. The importance of simplicity and clarity cannot be overemphasized.

Ifeanyi55 commented 1 year ago

I would like to take this issue. In what part of the doc exactly did you notice this? Is it in the Python client or Javascript client? @pngwn

freddyaboulton commented 1 year ago

This is specifically for the javascript client. EDIT: API info for the python client will be made better in v4

Ifeanyi55 commented 1 year ago

Nice, thanks @freddyaboulton

DarhkVoyd commented 10 months ago

Hey, I would like to take this issue if it's still valid.

pngwn commented 10 months ago

@DarhkVoyd definitely still valid! I'll add some additional details in a reply shortly!

pngwn commented 10 months ago

So I think the general idea was that it would be good to have response types typed, in order to do this we could need the client or the predict/ submit types to be generic.

Originally I was thinking that something like this would be cool:

import { client } from '@gradio/client';

const app = client(...)

type Input = ['textbox', 'number', 'radio'];
type Output = ['image', 'textbox'];

const resp = await app.predict<[Input, Output]>(...)

[!NOTE] The following types are illustrative, I haven't checked to see if they are correct.

the input would be typed as:

type Input = [string, number, string]

// type error
app.predict([1, 2, 3])

// correct
app.predict(["hi", 23, "cheese"])

response data would be typed as:

interface ImageData {
  path: string;
  name: string;
  url: string;
}

type TextboxData  = string;

type ResponseData = [ImageData, TextboxData];

// type error
response[2]
response[1].key
response[0].nsme
response[0].name.match(..) // regex method

// correct
response[0].name
response[0].name.toLowerCase()
response[1].substr(...) // string method

So essentially the input literals would become keys for specific types (that we would define somewhere) and we would map over those types to dynamically create the types for the inputs + returns values.

Additionally or optionally, I think it would be nice to be able to type the client directly, rather than needing to type every predict or submit call.

import { client } from '@gradio/client';

type PredictInput = ...
type PredictOutput = ...

type OtherInput = ...
type OtherOutput = ...

const ClientResponseTypes {
  "/predict" : [PredictInput, PredictOutput],
  "/other_endpoint": [PredictInput, PredictOutput],
}

I actually think the latter version (typing the client directly) is more universally useful, and we can also potentially generate the types via some command (we need this typing capability before we explore that though). So I would implement that first. We can always add additional stuff later.

Hope this is helpful. I would like to point out that we will be refactoring the client soon, it won't have enormous implications on this feature but it will result in some changes. Depending how soon you wanted to work on this, it might be better to wait until after that work has finished in order to avoid nasty conflicts. I also want to mention that this probably isn't the most straightforward of tasks, we already use generics extensively and this adds even more, they can be pretty tricky to work with for these kinds of purposes.

Hope this helps!

DarhkVoyd commented 10 months ago

@pngwn Hey, thank you so much for you insight. It's been a while so I just wanted to get right back. But I understand. I think I should just wait till the refactoring, I'll keep a watch. I would definitely tackle this carefully for the tricky part. Time to brush up my generics knowledge.

abidlabs commented 7 months ago

Closed via https://github.com/gradio-app/gradio/pull/7646! (Correct me if I'm wrong @hannahblair)

pngwn commented 7 months ago

This one is a slightly different issue. This is to add types for different component inputs + outputs, which we don't currently have types for.