Conversation
|
All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-2385-all-demos |
freddyaboulton
left a comment
There was a problem hiding this comment.
Thank you @abidlabs ! Looks good to me - left some minor suggestions.
I'll review the other PR once this one is merged.
| continue | ||
| block = self.blocks[output_id] | ||
| if getattr(block, "stateful", False): | ||
| if not utils.is_update(predictions[i]): |
There was a problem hiding this comment.
Unrelated to your PR but why do we not set the value of a state to be an update?
There was a problem hiding this comment.
Hmm good point. I suppose that if the update includes a value key, we should assign the value to the state.
I think this check was introduced because if a function returns values in a component-dictionary format (e.g. {textbox: "hi"}), we create placeholder updates for the rest of the components, and those placeholder components are a singleton dictionary: {"__type__"="generic_update"}. And we wouldn't want to assign this dictionary itself to the state.
Let me make a fix and add a test!
There was a problem hiding this comment.
It's fine if we keep it for now too - don't mean to slow down this PR. I was just curious hehe
There was a problem hiding this comment.
Ok actually gr.State() doesn't have an update() method at the moment, so passing in a dictionary of updates for gr.State() is not supported. There's no strong reason to add update() when a function can just return the value itself, so I'll leave this out for now.
|
Thanks for the review and great catches @freddyaboulton! Merging into the other branch |
* fixing examples * adding parameters * postprocess * examples * added tests * changelog * Update CHANGELOG.md * added docstrings * Adds ability to provide a `gr.update()` dictionary even if `postprocess=False` (#2385) * docs * fixes * added docs, fixed test * changelog * fix suggestoins * formatting * fix tests * fix tests
Previously, if
postprocesswasFalsein an event trigger, thegr.update()dictionary would not work. This PR fixes that and can be tested by running the example code in the issue:Fixes: #2378