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 **kwargs from queue #6232
Conversation
🪼 branch checks and previews
Install Gradio from this PR pip install https://gradio-builds.s3.amazonaws.com/425695d93556daa847a461f7938157867570c511/gradio-4.0.2-py3-none-any.whl Install Gradio Python Client from this PR pip install "gradio-client @ git+https://github.com/gradio-app/gradio@425695d93556daa847a461f7938157867570c511#subdirectory=client/python" |
🦄 change detectedThis Pull Request includes changes to the following packages.
With the following changelog entry.
Maintainers or the PR author can modify the PR title to modify this entry.
|
Why not raise an exception if we no longer support concurrency count? We deprecated it moving into 4.0. I think it's a good way to force old apps to think where they want to set their concurrency limits instead. |
You want to keep in signature but raise an error instead of warning? Happy to but |
I removed it from the signature because it was being deprecated, but I added **kwargs so that if an 3.x user tried to use it, they got a specific error telling them it was deprecated. (DeprecationWarning is an exception type, not a warning, despite its name). I feel like that's a proper deprecation compared to keeping it in the signature and it being ignored. |
…io into remove-deprecation-raise
It isn't deprecated if it is gone. A deprecation indicates that the thing still works but that it isn't recommended/ optimal/ may be removed in the future. I think this would be a |
I don't have a strong opinion either, but
This is the logic I used for getting rid of concurrency_count from the arguments. I didn't want users to see this as a valid parameter, but I still wanted to pass a specific message for users who tried using it. |
Good with this change. I agree that we should avoid |
Thanks for the discussion all! Keeping the warning raise but removing kwargs. |
Description
Don't think removing kwargs is a breaking change because we were already not allowing arbitrary kwargs. Removing them from the signature has the benefit of having your editor warn you before you run your code and reducing code for us. Also treat concurrency_count as a warning instead of an error.
🎯 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
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
You may need to run the linters:
bash scripts/format_backend.sh
andbash scripts/format_frontend.sh