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

Add upper bounds to all dependencies #544

Closed
adamjstewart opened this issue May 26, 2022 · 1 comment · Fixed by #557 or #1480
Closed

Add upper bounds to all dependencies #544

adamjstewart opened this issue May 26, 2022 · 1 comment · Fixed by #557 or #1480
Milestone

Comments

@adamjstewart
Copy link
Collaborator

Given the fallout of protocolbuffers/protobuf#10051, I think we should be specifying upper bounds for all of our dependencies. This can be done in several ways (see PEP 440 for syntax):

  1. pin patch version: torch==1.11.0
  2. pin minor version: torch==1.11.* or torch~=1.11.0 or torch>=1.11.0,<1.12
  3. pin major version: torch~=1.7 or torch>=1.7,<2

I strongly dislike 1 and 2 as they are overly restrictive and can lead to conflicts with other dependencies with slightly different version requirements. Of course, the benefit of 1 and 2 is that we've actually tested the exact versions of dependencies that we support.

I vote we go for 3. Of course, this syntax assumes that our dependencies actually know about and use semver, which may not always be the case. Although setuptools has been historically lax about this, flit and poetry have been much more proactive about recommending the use of things like ~= for semver-compliant dependencies.

This also relates to #488. #488 will make CI more stable, this issue relates more to our releases already on PyPI that may break when a dependency releases a new major version.

@adamjstewart
Copy link
Collaborator Author

I'm starting to rethink this decision. The vast majority of major bumps don't actually break backwards compatibility. The biggest offenders we've seen are torchmetrics and lightning, which have now reached 1.0 and 2.0 versions (which should theoretically imply stability). And they tend to first deprecate features before removing them, so we'll have plenty of time to push a new release before things impact users. Not pinning upper bounds should make patch releases easier since they will require fewer backports, and we'll avoid forgetting to do this like in #1232. I'm also influenced by the following blogs:

TL;DR: we should continue to follow SemVer as best we can, but we should not trust our dependencies to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant