Skip to content

Started updating demos to use the new gradio.components syntax#848

Merged
abidlabs merged 35 commits into
Blocks-devfrom
update-demos
Mar 23, 2022
Merged

Started updating demos to use the new gradio.components syntax#848
abidlabs merged 35 commits into
Blocks-devfrom
update-demos

Conversation

@abidlabs
Copy link
Copy Markdown
Member

@abidlabs abidlabs commented Mar 21, 2022

  • Updating all of the demos in the demos/ folder to use the new gradio.components imports and syntax
  • Creating new demos using Blocks
  • Also doing some minor cleanups as I encounter issues
  • As discussed with @pngwn, will add a script to quickly create a config for any particular demo from its name

omerXfaruq and others added 4 commits March 21, 2022 15:06
- default -> default_value refactoring
- refactor output types into output_type and make them auto
- all events are implemented
@abidlabs abidlabs requested a review from omerXfaruq March 21, 2022 20:07
@abidlabs abidlabs mentioned this pull request Mar 21, 2022
- refactor fn: str -> Callable
- add change event to TabItem
Comment thread gradio/__init__.py
import gradio.inputs as inputs
import gradio.outputs as outputs
from gradio.blocks import Blocks, Column, Row, TabItem, Tabs
from gradio.components import (
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.

@abidlabs are you sure about this? I think it is more proper to import components from gr.components. I am against importing all the components in the init. There are many components and a single component is not a critical element of gradio library, they need to referenced from gr.components

If people want to have shorter references they can import specific components via
from gradio.components import Textbox, Json...

Copy link
Copy Markdown
Member Author

@abidlabs abidlabs Mar 22, 2022

Choose a reason for hiding this comment

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

Hmm I really think the components and layouts both should be very easily imported since they are fundamental to building blocks. For me, gr.Textbox is a lot friendlier than gr.components.Textbox.

There are basically 3 ways to make it friendly that I considered.

  1. Doing from gradio.components import Textbox, Image... - the problem with this is that you have to explicitly import every component, which is annoying. Plus some of the components have common names (like Image), which could conflict with other classes in other common libraries (like from PIL import Image)
  2. Doing from gradio.components import * - bad practice, plus the same conflict issue
  3. Making the components accessible as gr.Textbox, etc. I think this is way to go. The only potential problem is that if we have conflicts within the gr. namespace, which is unlikely.

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.

Well I still think the same, might hear thoughts from @aliabid94 and @pngwn if it will help.

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.

We need to be able to support gr.Checkbox() or gr.Textbox(). It is way too much extra typing to have to include gr.components every single time. This is how streamlit does it as well. There are tradeoffs to be made between user-friendliness and proper explicitness in building APIs and gradio definitely needs to lean towards user-friendliness.

Comment thread gradio/__init__.py
import pkg_resources

import gradio.components as components
import gradio.inputs as inputs
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.

Why did you remove these? It makes gradio.inputs components unreachable from ide and user. They can import but cannot go to implementation.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

To be honest, I don't understand the effects of these lines. I thought importing a class in __init__.py just makes it accessible under gradio. namespace. But inputs and components are already accessible as gradio.inputs and gradio.components?

But if it is causing issues in your IDE, I can bring them back

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.

Yeah let's bring them back, this is what happens when we miss them:

image

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds good

omerXfaruq and others added 12 commits March 22, 2022 16:33
- Remove KeyValues from components
- Resolve Components imports in inputs and outputs
- fix default parameter usage in demos
- fix default parameter usage in demos
- fix default parameter usage in demos
- docstring updates
- add change event to Tabs
@abidlabs abidlabs mentioned this pull request Mar 22, 2022
abidlabs and others added 4 commits March 22, 2022 14:14
Base automatically changed from Blocks-Dev to Blocks-dev March 22, 2022 22:40
@abidlabs
Copy link
Copy Markdown
Member Author

Note: I've changed the non-keyword argument for the CheckboxGroup, Radio, and Dropdown components to be the choices instead of the default selected values. We should not require using a keyword to create a simple radio or dropdown or checkbox group. Now, this works as expected:

gr.Radio(["add", "subtract", "multiply", "divide"])

@abidlabs abidlabs changed the title Updating demos to use the new gradio.components syntax Started updating demos to use the new gradio.components syntax Mar 22, 2022
@abidlabs
Copy link
Copy Markdown
Member Author

Ok so I've just gone through the first 2 demos so far (and created 2 of my own). But opening this PR for review now so that it doesn't get too big!

@abidlabs abidlabs marked this pull request as ready for review March 22, 2022 23:55
@aliabid94
Copy link
Copy Markdown
Contributor

LGTM. What's the purpose of write_config.py?

@abidlabs
Copy link
Copy Markdown
Member Author

LGTM. What's the purpose of write_config.py?

Thanks. @pngwn needs it for setting up integration tests

@abidlabs abidlabs merged commit 398f556 into Blocks-dev Mar 23, 2022
@abidlabs abidlabs deleted the update-demos branch March 23, 2022 01:53
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.

3 participants