-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Integrating gradio demo #6034
base: master
Are you sure you want to change the base?
Integrating gradio demo #6034
Conversation
I have added the basic code but for some reason gradio app is not working. I will give it another try in a day or two. Another issue that I can use some help is, how to get hold of executor directly w/o creating the request and response objects. Once I have the input from the gradio app, I want a way to directly call the executor's main method. |
@@ -36,6 +36,7 @@ docarray>=0.16.4: core | |||
jina-hubble-sdk>=0.30.4: core | |||
jcloud>=0.0.35: core | |||
opentelemetry-api>=1.12.0: core | |||
gradio: core |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure if it is good to add this dependency as a core dependency. How big and how many extra dependencies does gradio bring?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, makes sense. Do you want me to add it some other type or remove it all together (and ask users to install it separately) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now lets ask users to install it, let think about it later then
@@ -1,5 +1,7 @@ | |||
import inspect | |||
from typing import TYPE_CHECKING, Callable, Dict, List, Optional, Union | |||
import gradio as gr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would imply thay impprting gradio should be protected, and the feature should be activated via an argument for Deployment or Flow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, I will move it inside the function call or if block
@@ -222,3 +233,62 @@ def _doc_to_event(doc): | |||
return {'event': 'update', 'data': doc.to_dict()} | |||
else: | |||
return {'event': 'update', 'data': doc.dict()} | |||
|
|||
def generate_gradio_interface(model: BaseModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these have to be more ambitious and also cover docarray built-in types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read that docarray is compatible with pydantic. Any thing that is different and you want me to pay special attention to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some special prebuilt types such as TextDoc, ImageDoc, etc ...
Codecov Report
@@ Coverage Diff @@
## master #6034 +/- ##
==========================================
- Coverage 77.58% 74.00% -3.59%
==========================================
Files 144 144
Lines 13788 13827 +39
==========================================
- Hits 10697 10232 -465
- Misses 3091 3595 +504
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Goals:
resolves #6026