gradio-app / gradio

Build and share delightful machine learning apps, all in Python. 🌟 Star to support our work!
http://www.gradio.app
Apache License 2.0
30.69k stars 2.28k forks source link

Fix bug in reload mode equality check. Better equality conversion for state variables #8385

Closed freddyaboulton closed 1 month ago

freddyaboulton commented 1 month ago

Description

Closes #8384

Fixes the linked issue specific reload mode. But the equality check for state, basically current_state != old_state would throw an error for some types like numpy/dataframe. So proposing a function to handle that case.

🎯 PRs Should Target Issues

Before your create a PR, please check to see if there is an existing issue for this change. If not, please create an issue before you create this PR, unless the fix is very small.

Not adhering to this guideline will result in the PR being closed.

Tests

  1. PRs will only be merged if tests pass on CI. To run the tests locally, please set up your Gradio environment locally and run the tests: bash scripts/run_all_tests.sh

  2. You may need to run the linters: bash scripts/format_backend.sh and bash scripts/format_frontend.sh

gradio-pr-bot commented 1 month ago

🪼 branch checks and previews

• Name Status URL
Spaces ready! Spaces preview
Website ready! Website preview
:unicorn: Changes detected! Details

Install Gradio from this PR

pip install https://gradio-builds.s3.amazonaws.com/7a875584690253279c7ac6a5261782de621fea3e/gradio-4.31.5-py3-none-any.whl

Install Gradio Python Client from this PR

pip install "gradio-client @ git+https://github.com/gradio-app/gradio@7a875584690253279c7ac6a5261782de621fea3e#subdirectory=client/python"

Install Gradio JS Client from this PR

npm install https://gradio-builds.s3.amazonaws.com/7a875584690253279c7ac6a5261782de621fea3e/gradio-client-0.19.4.tgz
gradio-pr-bot commented 1 month ago

🦄 change detected

This Pull Request includes changes to the following packages.

Package Version
gradio patch

With the following changelog entry.

Fix bug in reload mode equality check. Better equality conversion for state variables

Maintainers or the PR author can modify the PR title to modify this entry.

#### Something isn't right? - Maintainers can change the version label to modify the version bump. - If the bot has failed to detect any changes, or if this pull request needs to update multiple packages to different versions or requires a more comprehensive changelog entry, maintainers can [update the changelog file directly](https://github.com/gradio-app/gradio/edit/8384-state-equality/.changeset/ripe-tools-jam.md).
aliabid94 commented 1 month ago

Oh I'm hesitant about this one. The reason why reactive code blocks in svelte don't automatically do deep equality checks with lists/objects is that it could be extremely expensive accidentally. The problem in this PR is that we are going to be doing deep checks on any state change at all, even if there's no change event listener attached to the state object.

let developers define their own state equality check function

I think it is very likely that developers will see their performance degrade and not have any idea why this is happening.

I don't mind the deep check in reload mode, it's okay if we add lag on reload, but I'd feel more comfortable using deep checks for state changes only if we also only do state change checks if there is a change event listener for the state variable. That way the deep check is only happening when explicitly requested to.

freddyaboulton commented 1 month ago

Thank you @aliabid94 @abidlabs for the reviews!!