Skip to content

Embedding Components on Docs#1726

Merged
aliabd merged 18 commits into
mainfrom
embedded-components
Jul 19, 2022
Merged

Embedding Components on Docs#1726
aliabd merged 18 commits into
mainfrom
embedded-components

Conversation

@aliabd
Copy link
Copy Markdown
Contributor

@aliabd aliabd commented Jul 7, 2022

Embeds components on the docs page so readers can immediately see what they look like and interact with them.

The old way we did this was by hardcoding the configs, which meant they wouldn't change if the library did. This launches them the same way as demos. For most of the input components, the code is just this (to launch a demo with only the component):

import gradio as gr
with gr.Blocks() as demo:
    gr.[COMPONENT]()
demo.launch()

But some require parameters (like CheckboxGroup, etc) or some are output only. All the code to launch the demos is in /demo/components_demos/run.py, and modifying this will change the embedded component.

@aliabd aliabd requested a review from aliabid94 July 7, 2022 15:59
Copy link
Copy Markdown
Collaborator

@freddyaboulton freddyaboulton left a comment

Choose a reason for hiding this comment

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

Thank you so much for adding this back in @aliabd ! Already made a note of this in #1717 !

Comment thread website/demos/run_demos.py Outdated
assert "demo.launch()" in filedata, demo_name + " has no demo.launch()\n" + filedata
else:
os.mkdir(demo_folder)
filedata = component_launch_code.format(component_name=demo_name[:-10])
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be better if we defined a SUFFIX="_component" constant and then used it where it's needed.

Instead of demo_name.endwith("_component") we can do demo_name.endswith(SUFFIX) and .format(component_name=demo_name[:-len(SUFFIX)] as opposed to format(component_name=demo_name[:-10])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What will happen if we create a new component that requires constructor parameters? The demo will not work properly for sure, but will it break the website build?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To be clear, there isn't a component right now with required parameters. For example, CheckboxGroup should get choices but they're not required in the constructor, they default to None

So

import gradio as gr
with gr.Blocks() as demo:
     component = gr.CheckboxGroup()
demo.launch()

doesn't break the website, it just shows up like this:

Screen Shot 2022-07-08 at 5 50 52 PM

If we create a component with required parameters in the constructor, but don't override with a _component demo, it will break the website build.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok got it, I think we should try catch this to be more robust but otherwise LGTM!

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Jul 7, 2022

Wonder what people think about the _component folders being in /demos, is it going to confuse users?

@freddyaboulton
Copy link
Copy Markdown
Collaborator

@aliabd The _component folders are still demos so I think it's fine to keep in demos directory. Don't feel strongly about it though.

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Jul 8, 2022

@aliabd this looks great! But I don't see the embedded components on the preview website:

image

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Jul 8, 2022

@abidlabs there are a few of us who use the dev server now, I think when you checked someone else was testing a different change and checked out out of this branch. My mistake for linking to it as that's not how it should be used.

embedded-components-demo.mov

@abidlabs
Copy link
Copy Markdown
Member

abidlabs commented Jul 8, 2022

LGTM!

@aliabd
Copy link
Copy Markdown
Contributor Author

aliabd commented Jul 8, 2022

As suggested by @aliabid94, moved all the _component files to one file: components_demos.py

Comment thread website/demos/run_demos.py Outdated
def launch_demo(demo_folder):
subprocess.check_call([f"cd {demo_folder} && python run2.py"], shell=True)

SUFFIX = "_component"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

better name, COMPONENT_SUFFIX?

filedata = filedata.replace(f"demo.launch()", f"demo.launch(server_port={port}, _frontend=False)")
with open(demo_2_file, "w") as file:
file.write(filedata)
demo_thread = threading.Thread(target=launch_demo, args=(demo_folder,))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

some duplicated code across this if/else branches

};


// Remove built-with-gradio footers and extra space from embedded components
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

couldn't be done with css? also this needs to be done in guides too, maybe put this in a js file or common template so it can be used there too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will do this in a separate PR

Copy link
Copy Markdown
Contributor

@aliabid94 aliabid94 left a comment

Choose a reason for hiding this comment

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

LGTM beside small nits

@aliabd aliabd merged commit 0dbc8bf into main Jul 19, 2022
@aliabd aliabd deleted the embedded-components branch July 19, 2022 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants