Skip to content
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

Remove style parameter #4374

Merged
merged 27 commits into from Jun 8, 2023
Merged

Remove style parameter #4374

merged 27 commits into from Jun 8, 2023

Conversation

aliabid94
Copy link
Collaborator

@aliabid94 aliabid94 commented May 31, 2023

Wanted to get this cleanup out of the way before starting 4.0 stuff.

  • Fully deprecated .style paramater, moved arguments to constructor
  • .update supports all arguments formerly in style
  • scale and min_width can be applied directly to any component, rather than requiring a Column nesting that would break aligned widths and other issues.

Closes: #2395

@aliabid94 aliabid94 requested review from pngwn, freddyaboulton and dawoodkhan82 and removed request for pngwn and freddyaboulton May 31, 2023 09:36
@aliabid94 aliabid94 marked this pull request as draft May 31, 2023 09:39
@gradio-pr-bot
Copy link
Contributor

gradio-pr-bot commented May 31, 2023

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

@gradio-pr-bot
Copy link
Contributor

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

@pngwn
Copy link
Member

pngwn commented May 31, 2023

We will need to merge this into the 4.0 branch as it is a breaking change.

@aliabid94 aliabid94 marked this pull request as ready for review May 31, 2023 10:31
@aliabid94
Copy link
Collaborator Author

it's not breaking, it gives warnings but still works. Would be good to get this into main asap because it touches so many files, so it will get stale quickly.

@pngwn
Copy link
Member

pngwn commented May 31, 2023

Ah okay.

Copy link
Member

@pngwn pngwn left a comment

Choose a reason for hiding this comment

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

I tested as much as I could and lgtm! But we should wait until @dawoodkhan82 has tested as well.

CHANGELOG.md Outdated Show resolved Hide resolved
@abidlabs
Copy link
Member

abidlabs commented Jun 2, 2023

Some issues I noticed:

  • show_copy_button in Textbox not working:
import gradio as gr

def test(x):
    return x

with gr.Blocks() as demo:
    a = gr.Textbox(show_copy_button=True)
    b = gr.Textbox(show_copy_button=True)
    a.change(test, a, b)
    
demo.launch()
  • Setting container=False looks very ugly, is this expected?
import gradio as gr

def test(x):
    return x

with gr.Blocks() as demo:
    a = gr.Textbox(show_copy_button=True)
    b = gr.Textbox(container=False)
    a.change(test, a, b)
    
demo.launch()
image
  • height in Chatbot not having any effect:
import gradio as gr

def test(x):
    return x

with gr.Blocks() as demo:
    a = gr.Textbox(show_copy_button=True)
    b = gr.Chatbot(height=10)
    a.change(test, a, b)
    
demo.launch()
  • height and width in Image not having any effect:
import gradio as gr

def test(x):
    return "lion.jpg"

with gr.Blocks() as demo:
    a = gr.Textbox()
    b = gr.Image(height=10, width=200)
    a.change(test, a, b)
    
demo.launch()

Updated the parent comment to also say that this closes #2395 :)

@pngwn
Copy link
Member

pngwn commented Jun 3, 2023

Regarding #2395, do they actually work if you update them?

@aliabid94
Copy link
Collaborator Author

Fixed everything @abidlabs, can test with this:

import gradio as gr

with gr.Blocks() as demo:
    a = gr.Textbox(show_copy_button=True)
    b = gr.Textbox(show_copy_button=True)
    img = gr.Image(height=200, width=200)
    img_size = 200
    grow_btn = gr.Button("Grow Image")
    def grow_image():
        global img_size
        img_size += 100
        return gr.Image.update(width=img_size, height=img_size)
    grow_btn.click(grow_image, None, img)
    chat = gr.Chatbot([["hello", "hi"], ["hi", "hello"]], height=240)
    a.change(test, a, b)
    
demo.launch()

Regarding #2395, do they actually work if you update them?

Yes, can test using the demo above as well

@abidlabs
Copy link
Member

abidlabs commented Jun 7, 2023

Tested this pretty extensively, both the deprecated & new versions work quite well! A couple of small issues:

  • gr.Textbox() doesn't seem to support updating the style properties:
import gradio as gr

with gr.Blocks() as demo:
    c = gr.Textbox("cheetah.jpg", show_copy_button=True)
    demo.load(lambda :gr.update(show_copy_button=False), None, c)
    
demo.launch()
  • I am confused constructor parameters in gr.Gallery(). For example, the style parameters rows and columns have been renamed to grid_rows and grid_columns. Why? Better to keep the names the same in my opinion, they're simpler as they are. More importantly, the parameters seem to have no effect. Neither does preview nor update(). Am I missing something?
import gradio as gr

with gr.Blocks() as demo:
    c = gr.Gallery(["cheetah.jpg"]*6, preview=False, grid_cols=2)
    demo.load(lambda :gr.update(height=100), None, c)
    
demo.launch()

Good to go once these are fixed!

@aliabid94
Copy link
Collaborator Author

gr.Textbox() doesn't seem to support updating the style properties:

Fixed

am confused constructor parameters in gr.Gallery(). For example, the style parameters rows and columns have been renamed to grid_rows and grid_columns. Why? Better to keep the names the same in my opinion, they're simpler as they are. More importantly, the parameters seem to have no effect. Neither does preview nor update(). Am I missing something?

Fixed and changed names.

@aliabid94 aliabid94 merged commit ff6e676 into main Jun 8, 2023
19 of 20 checks passed
@aliabid94 aliabid94 deleted the fix_layouts branch June 8, 2023 01:35
This was referenced Jun 27, 2023
Keldos-Li added a commit to GaiZhenbiao/ChuanhuChatGPT that referenced this pull request Jul 9, 2023
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.

Allow gr.update() to accept a component's .style() parameters?
5 participants