gradio icon indicating copy to clipboard operation
gradio copied to clipboard

Python backend to theming

Open aliabid94 opened this issue 1 year ago • 18 comments

Created python backend that will allow users to create and modify backends from python. User's can specify existing themes as:

with gr.Blocks(theme=gr.themes.Solid) as demo:
    pass

and they can modify themes, with full type hints, via:

theme = gr.themes.Default()
theme.color_focus_primary = "#aaaaaa"

with gr.Blocks(theme=theme) as demo:
    pass

Here's how it works:

  1. Themes are stored in gradio/theming/configs as jsons
  2. gradio/theming/_gen_themes.py converts these jsons into python classes. Each css variable is converted into an attribute of the class
  3. These python classes can be imported, modified, etc.
  4. When passed a Theme class, Blocks will generate the css from the attributes.
  5. This css file will be accessed via localhost:7860/theme.css

Currently the best way I could think of implementing, lmk what you guys think.

Example running demo/blocks_xray below: Recording 2023-01-04 at 18 29 06

(for some reason, vscode isn't type hinting the theme attributes for me, will figure out. For now looking for feedback on general implementation_

Closes: #2354

aliabid94 avatar Jan 05 '23 00:01 aliabid94

The demo notebooks don't match the run.py files. Please run this command from the root of the repo and then commit the changes:

pip install nbformat && cd demo && python generate_notebooks.py

github-actions[bot] avatar Jan 05 '23 00:01 github-actions[bot]

This generally looks good to me @aliabid94! One thing to keep in mind is loading external themes, i.e. to make it possible for users to publish themes and load each other's themes. (We could use Spaces as our hosting/versioning system for hosting the theme files, which would give us several advantages like automatic versioning & potentially the ability to "preview" a theme simply by opening the Space).

How could we load a theme from Spaces with this API? Perhaps it would be something like:

gr.themes.load("gradio-themes/grass")

Or even simpler:

gr.theme("gradio-themes/grass")

For the sake of consistency, perhaps we should do the same with all of our themes:

with gr.Blocks(theme=gr.theme("default")) as demo:
    pass

In other words, gr.theme() could take in a built-in theme or an external theme as argument, and would return a gr.Theme object as you have proposed

abidlabs avatar Jan 08 '23 09:01 abidlabs

🎉 The demo notebooks match the run.py files! 🎉

gradio-pr-bot avatar Jan 17 '23 23:01 gradio-pr-bot

All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2931-all-demos

gradio-pr-bot avatar Jan 17 '23 23:01 gradio-pr-bot

Been taking frustratingly long to get this across the finish line, but I think it would be great to get your guys' thoughts on the general implementation @pngwn @freddyaboulton @abidlabs while I finish fixing up the css variable values.

This PR kind of grew past the scope I was intending, but in this PR:

  • Have moved all the CSS variables to a Theme class object that generates a theme.css file
  • Themes mainly configure colors, roundedness, and font
  • Have refactored the variables to remove all "component specific" variable types, instead using general color and text controls that all components must use
  • Have created a Default theme with quickly modifiable color values from the constructor
  • Have allowed user to specify any font that will automatically be loaded from Google Fonts (right now does not work in frontend dev mode. also doesn't allow user to specify system fonts instead of google fonts yet)

aliabid94 avatar Jan 25 '23 10:01 aliabid94

Have allowed user to specify any font that will automatically be loaded from Google Fonts (right now does not work in frontend dev mode. also doesn't allow user to specify system fonts instead of google fonts yet)

I wouldn't do this right now, because it will make offline mode even more broken and I don't think we should spend the effort when we'll need to redo it shortly.

I'll take a look at the general implementation tomorrow and leave some comments.

pngwn avatar Jan 25 '23 16:01 pngwn

Ok sure, I can take the font stuff out. I do think fonts are a pretty big part of theming though and we should bring them in soon

aliabid94 avatar Jan 25 '23 19:01 aliabid94

Ready for review! I rolled back the font changes and preset themes other than Default.

aliabid94 avatar Jan 27 '23 00:01 aliabid94

Thanks @aliabid94! This looks very intriguing. The deployed PR Space looks pretty good: https://huggingface.co/spaces/gradio-pr-deploys/pr-2931-all-demos, except for a few things:

Border of radio buttons and checkboxes have disappeared

Main

image image

This branch

image image

Colors of the button are more washed out

Main

image

This branch

image

The label percent bars have disappeared

Main

image

This branch

image

I'm not sure what else I should review for this PR? Can you provide some example code to test the Python backend?

abidlabs avatar Jan 27 '23 06:01 abidlabs

will fix above. If you have thoughts on the implementation, that's probably the most important discussion

aliabid94 avatar Jan 27 '23 07:01 aliabid94

In line 330 of interface.py, the lines:

        if not (self.theme == "default"):
            warnings.warn("Currently, only the 'default' theme is supported.")

need to be removed, otherwise a warning happens if you pass in a theme to an Interface like this:

demo = gr.Interface(
    generate_text, 
    "textbox", 
    "textbox", 
    theme=gr.themes.Default()
).launch()

abidlabs avatar Feb 01 '23 00:02 abidlabs

Ok @aliabid94 so this is a really awesome PR and it's allowed me to create some flashy themes very quickly:

image

However, I do have some suggestions.

Styling:

  • I don't think the footer links color should inherit from neutral_hue. I'm pretty sure users will rarely want them to change from their default gray color to something like this:
image
  • In dark mode, the Gradio app appears on a black background if the neutral_hue is changed:
image

abidlabs avatar Feb 01 '23 00:02 abidlabs

Overall, the Python API is quite nice! There are a lot of attributes so it would be really nice to have docs / interactive playground to know what properties to adjust but I like the way everything is structured.

I think we should definitely create another theme class (perhaps a gr.themes.Minimalist that mimics the SD space?) to test it out more fully

abidlabs avatar Feb 01 '23 00:02 abidlabs

Awesome work. I think it looks good for the most part.

I generally think the API is good (simple and intuitive for users) but i'm not a huge fan of mutating the theme instance directly:

Instead of this:

theme = gr.themes.Default()
theme.color_focus_primary = "#aaaaaa"

I would rather do this:

theme = gr.themes.Default({ 
  "color_focus_primary":  "#aaaaaa"
})

The record passed forms a set of overrides ti the base theme that is selected. I think this preferably for two reasons. Firstly iy is easier to reason about, avoiding mutation in this case makes more sense because the theme has to 'set' before it is passed into the Block/ Interface.

The ambiguity of the mutable API makes me think this should do something but it won't (i think):

theme = gr.themes.Default()

with gr.Blocks(theme=theme) as demo:
    pass

theme.color_focus_primary = "#aaaaaa"

By passing the overrides as an argument, the ambiguity is removed and the API is easier to understand.

The second reason I think it is a better choice is because I think the API for local themes vs remote themes should essentially be identical (gr.themes.Default() vs gr.themes.load("...")). Passing an argument for overrides allows us to keep things consistent across 'theme loader' APIs and gives more flexibility in how we manage the work. If we wanted to store the overrides and deal with them later so we can do more work concurrently then this API allows for that but if we want the theme instance to be mutable immediately after it is 'called' then we have to complete that work before we do anything else.

Regarding Dark mode + light mode

@freddyaboulton They need to be part of the same theme for two reasons.

Firstly, pit of success. Make the right thing easy and the wrong thing hard. We should be encouraging people to create light and dark mode themes because it isn't just an issues of stylistic preference. Separating them makes it too easy to 'skip' whatever mode you don't care about. It will always be possible to just write a single theme, the terminology being used regarding light/dark mode is actually incorrect. The more accurate description is 'base' and 'dark'. The base theme applies all of the time and is the light mode theme but also acts as a default, the darkmode theme does not necessarily override every single value, just those values that need to change (typically colours and not every colour). It only applies in dark mode.

So every them them has a base theme and a set of dark mode over-rides rather than them being distinct themes.

The second reason is that consuming a community theme would be very painful for users. As a user choosing a high quality theme, I would be looking for good light and dark modes. If I then had to load/import them separately, that would be frustrating. It also introduces more opportunities for error and issues with naming collisions in our remote host. A user could easily make a typo in or the other theme. That typo might even be a legit theme but the incorrect one. Likewise what if the dark variant theme is taken for whatever reason, we can't guarantee that every dark variant will be <theme_name>-dark. Sharing themes would also be annoying. Authors would need to upload their them twice to different namespaces, have duplicate docs and do all of the work of informing users that it is only a part of the whole theme. It would also be impossible to version the themes together, which is something that we should encourage as a best practice.

Thoughts about schema

We need a language agnostic representation of the theme if we want to be able to use the generated themes in different contexts and map them to CSS values (i.e. a theme builder in the web) or back to the python. For example, the Python will need to generate 'something' we can use in the web, we could use the raw CSS for that but in order to manipulate the actual values we would have to parse all of the CSS to do it. Likewise when we want to generate a python theme from the CSS value in memory we need a way to map them to the Python class. Designing this up front (along with contracts between the agnostic schema _ css/py representations) will be much simpler in the long run.

Nits

I don't understand why radius was changed to rounded i also don't understand why the size modifier was removed from medium. They are tailwindisms and unnecessary. radius is perfectly understandable and maps clearly to the CSS spec, changing it to a non-standard word is just 'one more thing' to learn for people already familiar with CSS. radius is also the terminology used in all graphics editing software and is the proper term for this feature of a rectangle. It also marks a difference between the CSS framework we are using (which uses radius) and our own API, increasing the maintenance burden for no benefit. The removal of the md modifier for medium introduces unnecessary ambiguity.

row-even-background is incorrect. It specifically refers to tables, not any row. We should add the table qualifier back or come up with a clearer term, we have multiple concept of rows in Gradio and this is unnecessarily abiguous.

No opinion of soft/light.

The hue implementation will not work. The human eye perceives different hues very differently so we will need to select the correct shades for different accent hues by hand in order to produce a high quality result. This especially true when selecting light and dark mode shades. We simply cannot automate them in this way.

Question

@aliabid94 How do you actually write a theme from scratch? Do you have to write a file like themes/Default.py?

pngwn avatar Feb 01 '23 14:02 pngwn

THanks for the review @pngwn. Just to go through your feedback:

I would rather do this:

theme = gr.themes.Default({ 
  "color_focus_primary":  "#aaaaaa"
})

I can move the theme properties to the constructor, that way we can get typehinting such as this:

theme = gr.themes.Default(color_focus_primary="#aaaaaa")

Thoughts about schema

We need a language agnostic representation of the theme if we want to be able to use the generated themes in different contexts and map them to CSS values (i.e. a theme builder in the web) or back to the python. For example, the Python will need to generate 'something' we can use in the web, we could use the raw CSS for that but in order to manipulate the actual values we would have to parse all of the CSS to do it. Likewise when we want to generate a python theme from the CSS value in memory we need a way to map them to the Python class. Designing this up front (along with contracts between the agnostic schema _ css/py representations) will be much simpler in the long run.

I think this is something we can address outside this PR. We can sync on how we want themes to be shareable, etc.

I don't understand why radius was changed to rounded i also don't understand why the size modifier was removed from medium. They are tailwindisms and unnecessary. radius is perfectly understandable and maps clearly to the CSS spec, changing it to a non-standard word is just 'one more thing' to learn for people already familiar with CSS. radius is also the terminology used in all graphics editing software and is the proper term for this feature of a rectangle. It also marks a difference between the CSS framework we are using (which uses radius) and our own API, increasing the maintenance burden for no benefit. The removal of the md modifier for medium introduces unnecessary ambiguity.

I want users to be able to control the roundedness in themes. The reason behind having rounded be a separate CSS variable from radius is that users can specify how round they would like rounded or rounded-lg to correspond to. Previously, we hardcoded roundedness using --radius-X variables that come from pollen directly, which would not allow users to control the roundedness in their themes.

row-even-background is incorrect. It specifically refers to tables, not any row. We should add the table qualifier back or come up with a clearer term, we have multiple concept of rows in Gradio and this is unnecessarily abiguous.

Okay sure will revert.

The hue implementation will not work. The human eye perceives different hues very differently so we will need to select the correct shades for different accent hues by hand in order to produce a high quality result. This especially true when selecting light and dark mode shades. We simply cannot automate them in this way.

Isn't the idea of these numeric color schemes that orange-600 corresponds to blue-600, and they are interchangeable? It would be nice if users can just change the primary color of a theme without having to edit every single colored attribute imo. And if you try inputting different hues for primary_hue in the Default theme for example, they look good.

Question

@aliabid94 How do you actually write a theme from scratch? Do you have to write a file like themes/Default.py?

Right now, yes. We can consider other approaches in another PR.

aliabid94 avatar Feb 01 '23 20:02 aliabid94

I want users to be able to control the roundedness in themes. The reason behind having rounded be a separate CSS variable from radius is that users can specify how round they would like rounded or rounded-lg to correspond to. Previously, we hardcoded roundedness using --radius-X variables that come from pollen directly, which would not allow users to control the roundedness in their themes.

There is no reason why they can't just have the same name, since it is already a sensible name. CSS variables can be overriden at will.

:root {
  --color: blue;
  --color: pink;
}
	
h1 {
  color: var(--color)
}

This is perfectly valid, in fact is an intended part of the design. In this instance I would just keep the same name and if there is no value passed in, set it to None (in which case the theming implementation already skips generating a variable for them). In this case if a theme doesn't pass in a value then the pollen values will be used, if a user does then a new css variable will be generated and those values will take precedence. If they don't then it is just a bug due to the order the variables are defined in needs fixing regardless.

Isn't the idea of these numeric color schemes that orange-600 corresponds to blue-600, and they are interchangeable? It would be nice if users can just change the primary color of a theme without having to edit every single colored attribute imo. And if you try inputting different hues for primary_hue in the Default theme for example, they look good.

In principle yeah, but in reality I've had mixed experience with that. Totally agree with the goal, just uncertain about the implementation. We can leave them for now and see if we come across any issues. I'll test all possible colours values thoroughly to see what the output looks like.

I can move the theme properties to the constructor, that way we can get typehinting such as this:

I've thought about this some more and I actually think the 'namespacing' that was in the original JSON proposal is really beneficial here. I don't how much we need but i feel like passing in a huge list of x_y_z kwargs is little better than writing pure CSS (maybe even worse since is a very thin abstraction). Sure you get typehints but there will be so many that I'm not sure how useful it will be. So in terms of an API for applying on top of a base, I'm not really convinced it is the best option.

I'm amazed that you can't properly type args and get vscode benefits:

class Color(TypedDict):
  body: str
  cta: str
  other: str

class Size(TypedDict):
  sm: int
  md: int
  lg: int

class ThemeDict(TypedDict):
  color: Color
  spacing: Size
  radius: Size


class Theme():
    def __init__(
        self,
        config: ThemeDict
    ):
        pass

Shouldn't this at least work in some way? I think the code is correct but i don't get anything helpful from VSCode at all other than this:

Screenshot 2023-02-02 at 10 14 30

I'm not even sure why this feature exists if it does nothing useful.

pngwn avatar Feb 02 '23 10:02 pngwn

I can move the theme properties to the constructor, that way we can get typehinting...

This makes sense, although it would cause the number of parameters in the constructor to dramatically increase, and I wonder if it would make it harder to "quickly get started" with a theme. And what if someone provides arguments to both primary_hue and color_focus_primary -- which one takes precedence?

I wonder if there should two different methods, one to quickly create themes based on providing high-level properties, and another one that lets you set all of these properties individually for different themes.

abidlabs avatar Feb 02 '23 16:02 abidlabs

Right now, yes. We can consider other approaches in another PR.

The reason I asked about this is because I think the API for creating a theme vs modifying an existing one should be similar (if not the same). So I think it is worth thinking about it now.

pngwn avatar Feb 02 '23 16:02 pngwn

Theming ready for review! New API:

  • Import themes from gradio.themes and set them in Blocks/Interface constructor, e.g. gr.Blocks(theme=gr.themes.Soft())
  • Themes have six constructor arguments for high-level control of look and feel.
    • Colors: primary_hue, secondary_hue, and neutral_hue. Each of these is a gr.themes.utils.Color object, which is a palette ranging from 50-900
    • Sizes: spacing_size, text_size, radius_size. Each of these is a gr.themes.utils.Size object, which a size list ranging from xxs to xxl.
    • For example, gr.themes.Default(primary_hue=gr.themes.utils.green)
  • The ~200 theme variables can be set via the set method
    • For example, gr.themes.Default(primary_hue=gr.themes.utils.green).set(block_background="blue")
    • Theme variables can refer to each other, or the six constructor arguments, with an asterisk prefix.
    • For example, gr.themes.Default(primary_hue=gr.themes.utils.green).set(block_background="*primary_500", checkbox_label_background="*block_background")
  • Dark mode variables have a _dark suffix
    • For example gr.themes.Default(primary_hue=gr.themes.utils.green).set(block_background="*primary_500", block_background_dark="*neutral_800")

Also created the blocks_kitchen_sink demo. which showcases all Gradio features, and easy controls to toggle between themes and dark mode.

themes

aliabid94 avatar Mar 02 '23 01:03 aliabid94

Looks really good @aliabid94! My one feedback (copied from Slack) is:

The theming works really well, no issues I could find. My main pain point was importing the colors and the sizes for the constructor. It would be really, really nice if you could pass in string shortcuts for all of the arguments, and they get mapped appropriately: "blue" --> gr.themes.utils.blue "sm" --> gr.themes.utils.size.radius_sm and so on

Let's merge this in after we've released 3.20!

abidlabs avatar Mar 03 '23 01:03 abidlabs

I noticed missing is the comment about queue=False

Sorry, what comment?

and not deleting style prop from the component. Not sure what you and @dawoodkhan82 agreed to.

Restored the style prop to chatbot (without color_map)

If color_map is deprecated, is there a way to target the chatbot bubble colors with the python theming api? I don't think so but want to make sure.

The colors for the chatbot inherit from theme values as such:

.bot,
.pending {
  border-color: var(--color-border-primary);
  background-color: var(--color-background-secondary);
}
.user {
  border-color: var(--color-border-accent);
  background-color: var(--color-accent-soft);
}

Users can modify these theme variables, or for more control, hijack with the CSS kwarg.

aliabid94 avatar Mar 06 '23 18:03 aliabid94

@aliabid94 Thanks for responding + clarifying the new css variables for the chatbot!

That comment was meant to go to this PR https://github.com/gradio-app/gradio/pull/3370 lol sorry about that 🙈

This PR looks good to me though!

freddyaboulton avatar Mar 06 '23 18:03 freddyaboulton

Thanks everyone! Finally merging

aliabid94 avatar Mar 06 '23 20:03 aliabid94