-
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
Callable blocks #1437
Callable blocks #1437
Conversation
I have installed this branch locally and got this issue while testing it with Dalle-Mini:
|
It did work with CLIP Guided Faces though:
Works (loading, haven't tested executing it yet) However, would it be possible to keep the mini-auto-documentation feature of 2.x? Having some way to print or list each of the I was thinking something equivalent/same to 2.x loaded spaces, that if you print the function generated it creates this
|
Good idea @apolinario, I'll add this functionality, and look into why dalle-mini isn't working (there are sooo many cases to consider lol) |
Ok so the reason that It just uses the Gradio components on the frontend and then makes an API request in javascript. As a result, it can't really be used with |
Also checked loading and using the top Spaces of the Week (see below) and they seemed to work well. So ready for review @aliabid94!
|
I get the error |
So like I mention in the first example, if you are not in an async function, you will need to do something like this: import asyncio
import gradio as gr
io = gr.Interface.load("spaces/abidlabs/blocks-dev-test")
asyncio.run(io("a", fn_index=0)) because under the hood, everything is async |
After discussing with @aliabid94, decided to make the calls synchronous instead of asynchronous. This drops support for async functions when using I've updated the usage examples above |
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.
LGTM after addressing/fixing comments
@@ -310,8 +312,91 @@ def iterate_over_children(children_list): | |||
event_method = getattr(original_mapping[target], trigger) | |||
event_method(fn=fn, **dependency) | |||
|
|||
# Allows some use of Interface-specific methods with loaded Spaces |
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.
Instead of implementing Interface specific things in Blocks, can you overwrite __call__ in Interface?
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.
You can call super().__call__ where necessary
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 code is in from_config()
not __call__()
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.
In this case, I don't think it's actually appropriate to put it in Interface. Because it's actually a Blocks
that gets created, not an Interface
. This additional hack simply lets the Blocks
be used in some ways that Interfaces
are used -- it's more or less an experimental thing, not necessarily something we want to support fully right now.
processed_input = self.preprocess_data(fn_index, serialized_params, None) | ||
|
||
if inspect.iscoroutinefunction(block_fn.fn): | ||
raise ValueError( |
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.
@abidlabs why not run async function in asyncio.run()
, I think we should support async functions as well, or this feature is incomplete.
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.
The problem is that this approach wouldn't work inside of a jupyter notebook. You get this error: https://stackoverflow.com/questions/55409641/asyncio-run-cannot-be-called-from-a-running-event-loop
I wasn't sure how to fix it myself, so I left this case as unsupported for now
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.
hmm, today I looked into a little bit, it is a bit problematic case, it might be solved with fsspec library, will look into it in the future!
This PR allows
Blocks
to be used as regular functions, just likeInterfaces
. This feature will be much nicer to use once we haveapi_names
(PR #1397), but this PR sets up the basic functionality in place.Notice that calling the function returns an async coroutine so you must await it or async.run it. The reason for this is now we support async functions inside Blocks, so everything is treated asynchronously. This is also nice because you can call multiple api requests concurrently (yay parallelization!)
fn_index
-- here's example usage:This is a super simple Space that has 2 functions, one that doubles the word, and one that triples the word. You can call each of the functions by doing:
Right now, this requires you to know the
fn_index
for each function. This will be nicer when we can use theapi_name
instead. cc @apolinarioThis PR also does two other things:
@dawoodkhan82 and I discovered a bug with image uploads. It fixes that in
Upload.svelte
It addresses Gradio 3.0 cannot load interfaces from Gradio 2 #1417 by making somewhat of an assumption (that people will only use
gradio.mix
if they load anInterface
, and that for anInterface
, the first blocks function is the prediction function). Although this may not always be True, the use case in Gradio 3.0 cannot load interfaces from Gradio 2 #1417 is important enough such that we should support it (I've added warnings to be safe). You can test the code sample in Gradio 3.0 cannot load interfaces from Gradio 2 #1417 to make sure it works:cc @osanseviero
Closes: #1417