-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Pinning dependencies in requirements.txt #4885
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
All the demos for this PR have been deployed at https://huggingface.co/spaces/gradio-pr-deploys/pr-4885-all-demos You can install the changes in this PR by running: pip install https://gradio-builds.s3.amazonaws.com/d48f463cd4f8745b687b9fd5a96bbcb245cbf101/gradio-3.36.1-py3-none-any.whl |
🎉 Chromatic build completed! There are 0 visual changes to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for going through this abidlabs. Since we're only pinning to majors that were released at least a couple of years ago I think this will not cause unintended upgrade/downgrades of dependencies in developer's environments. Would be good to communicate to developers that we're making this change though?
requirements.txt
Outdated
packaging | ||
pandas>=1.0,<3.0 | ||
pillow>=8.0,<11.0 | ||
pydantic>=1.0,<3.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Let's use the same pin as fastapi. Version 1.0 is actually not allowed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. Do we have to do this? I don't think we need to use the same versions of fastapi (that's a bit redundant anyways since fastapi is one our dependencies). We just need to make sure that our library's use of pydantic
is valid according to the version of pydantic
we've pinned.
That being said, I haven't tested different versions of pydantic
so maybe we actually do need to be more strict
Yeah good point, I'll add a note in the breaking changes, and let's post on Twitter + Discord when we do the next release. |
Was there an issue or something tracking why this is needed for so many packages? Overly tight pins will cause issues in downstream projects utilizing Gradio – and furthermore, not all Python packages do Semver at all, so pinning to a major version may not mean what you expect. |
Hi @akx yes, the issue was that pydantic released 2.0, which included breaking changes that broke a large number of Gradio apps. See #4835 I agree that pinned dependencies can cause downstream conflicts, which is why we've kept the pins pretty loose -- it's mostly future-proofing. We'll need to keep an eye out for new releases of our dependencies and update pins as needed, but we're expecting this PR to be better for the long-term stability of Gradio apps. |
Updated pins and improved changelog message @freddyaboulton -- I think this PR should be good to go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the nice PR @abidlabs !!
For
gradio
, here is what I did with each external dependency:/file
route works with audio files for this version of aiofiles so I allow 22.x, 23.xgr.Markdown
andgr.DataFrame
in the frontend, likegr.Chatbot
#4523gr.load()
works with tabular examples with 5.x, 6.x so I pin thoseFor
gradio_client
:gradio
gradio
packaging
Separately, I also added:
as requirements to
gradio
since we use them explicitly in thegradio
source code. Safer to include them rather than depend on them being available fromgradio_client
since the client source code may change.