-
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
Render decorator 2 #8110
Render decorator 2 #8110
Conversation
@aliabid94 very cool PR! I'm testing this out and will let you know about any issues I find. One issue I noticed so far is that if I have two Blocks defined in the same Python session, this does not work. I originally noticed this in the context of running multiple demos in different cells in a jupyter notebook, but you can reproduce with this example: import gradio as gr
with gr.Blocks() as demo:
pass
with gr.Blocks() as demo:
input_text = gr.Textbox(label="input")
mode = gr.Radio(["textbox", "button"], value="textbox")
@gr.render(inputs=[input_text, mode], triggers=[input_text.submit])
def show_split(text, mode):
if len(text) == 0:
gr.Markdown("## No Input Provided")
else:
for letter in text:
if mode == "textbox":
gr.Textbox(letter)
else:
gr.Button(letter, mode)
demo.launch() If I remove the first Blocks, it works fine, with the first Blocks, I get infinite loading after submitting the textbox: |
I noticed that you got rid of the behavior where if a |
Otherwise working quite well in my tests @aliabid94! So the plan is to publicize / document this after the next PR is in? |
data: { | ||
components: any[]; | ||
layout: any; | ||
}; |
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.
We could use these types from js/app/src/types.ts
data: { | |
components: any[]; | |
layout: any; | |
}; | |
data: { | |
components: ComponentMeta[]; | |
layout: LayoutNode; | |
}; |
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 don't think we can import from js/app into client, because that would be a circular dependency given we import client into js/app? At least that's why I've assumed that we redefined Dependency and Config in both js/app/src/types.ts and client/js/src/types.ts. We also marked Config.components as an any
type in client/js/src/types.ts instead of using ComponentMeta.
Agree that these definitions should all be merged at some point.
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.
Just one small comment, but otherwise looks good! Thanks @aliabid94!
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.
Very clever PR @aliabid94 ! I tested some demos and its working well. I have two comments based on my tests:
- Be able to replace the contents of an existing context. Right now, the output of
render
is always appended to the current context where it's defined but I think it will be useful to be able to replace the contents of the context completely. For example, I wanted to modify therender_split
demo to add a placeholderMarkdown
component explaining what would happen. I can't get therender
to replace that placeholder text.
import gradio as gr
with gr.Blocks() as demo:
input_text = gr.Textbox(label="input")
mode = gr.Radio(["textbox", "button"], value="textbox")
with gr.Tabs():
with gr.Tab("Tab 1"):
gr.Markdown("# Input some text in the textbox and see the text split into individual components")
@gr.render(inputs=[input_text, mode], triggers=[input_text.submit])
def show_split(text, mode):
if len(text) == 0:
return
else:
for letter in text:
if mode == "textbox":
gr.Textbox(letter)
else:
gr.Button(letter)
if __name__ == "__main__":
demo.launch()
- You can't update a component and render. I can imagine use cases where you would want to render some new components and update existing ones. I can't get that to work.
render
has nooutputs
parameter so I tried defining a separate event trigger with the same trigger as the render. But the render will not be reflected in the UI in that case.
For the first point, maybe we can add a parameter to render
to control that behavior? For the second point, I think it's fine if render
has no outputs
parameter and it's only for declaring UI. But we should respect both the render
and any additional triggers.
EDIT: Made a mistake in my first comment
Fixed
Sure I can add this support in another PR, I'm going to add all convenient shorthands in another PR.
Both of could be handled by putting the "existing context" inside the render block, right? Will add support for allowing render context to be run on load (e.g. @gr.render(inputs=[input_text, mode], triggers=[demo.load, input_text.submit])
def show_split(text, mode):
if len(text) == 0:
gr.Markdown("# Input some text in the textbox and see the text split into individual components")
else:
for letter in text:
if mode == "textbox":
gr.Textbox(letter)
else:
gr.Button(letter) If you really want to update other components outside the render context, just create a separate event listener. I think combining these into one API function is kinda messy. |
Yea agreed. I tried that yesterday and it didn't work though. I had a separate |
@freddyaboulton can you provide a repro? This works for me: import gradio as gr
with gr.Blocks() as demo:
input_text = gr.Textbox(label="input")
mode = gr.Radio(["textbox", "button"], value="textbox")
with gr.Tabs():
with gr.Tab("Tab 1"):
md = gr.Markdown("# Input some text in the textbox and see the text split into individual components")
@gr.render(inputs=[input_text, mode], triggers=[input_text.submit])
def show_split(text, mode):
if len(text) == 0:
return
else:
for letter in text:
if mode == "textbox":
gr.Textbox(letter)
else:
gr.Button(letter)
input_text.submit(lambda: gr.Markdown(visible=False), None, md)
demo.launch() |
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 @aliabid94 !!
Reproduced myself (it's working). Just wanted to say this stuff is amazing and so, so timely :) Will this go in the next major release and if so, what's the timeline for when that will be released? Would be happy to write docs etc. if that will speed it up. |
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.
Very cool!! Everything looks good on my side
Thanks @jameszhou02! This is actually the first of a two-part PR, we are planning on adding this to the docs after the second PR is in. In the meantime, if you'd like to use it, you can install directly from this PR:
|
This code raises errors for me. It prevents rendering entirely. Wrapping a block around it doesn't seem to change anything. import gradio as gr
with gr.Blocks() as demo:
input_text = gr.Textbox(label="input")
mode = gr.Radio(["textbox", "button"], value="textbox")
@gr.render(inputs=[input_text, mode], triggers=[input_text.submit])
def show_split(text, mode):
if len(text) == 0:
gr.Markdown("## No Input Provided")
else:
for letter in text:
if mode == "textbox":
gr.Textbox(letter)
else:
btn = gr.Button(letter, mode)
btn.click(lambda: print(letter), inputs=[], outputs=[])
demo.launch()
|
Support for adding event listeners inside render contexts is coming up in a follow up PR! |
Round 2 with the render decorator! See demo/render_split, and demo/render_merge for examples. Will support event listeners involving rendered elements in next PR.
demo/blocks_split
Demonstrates the ability to render elements dynamically based on logic:
demo/blocks_merge
Demonstrates the ability to render input elements dynamically while preserving values using key= arg: