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

Relax upper bounds on dependencies #227

Closed
TomAugspurger opened this issue Feb 7, 2022 · 4 comments · Fixed by #228
Closed

Relax upper bounds on dependencies #227

TomAugspurger opened this issue Feb 7, 2022 · 4 comments · Fixed by #228
Labels
bug Something isn't working
Milestone

Comments

@TomAugspurger
Copy link
Collaborator

TomAugspurger commented Feb 7, 2022

Currently, all the stactools dependencies are set as ~= dependencies:

stactools/setup.cfg

Lines 33 to 42 in 4a19089

install_requires =
Shapely ~= 1.7
aiohttp ~= 3.7
click ~= 8.0
fsspec ~= 2021.7
lxml ~= 4.6
pyproj ~= 3.0
pystac[validation] ~= 1.2
rasterio ~= 1.2
requests ~= 2.25
. Would folks be OK with removing those upper bounds?

For some packages that follow calver, like fsspec, that kind of constraint isn't meaningful. There's no reason to pin to a specific month's release.

It's also not meaningful for packages that use x.y.z version numbers, but don't actually promise to follow semver. I haven't checked if all those libraries declare that they do.

I also don't think it's appropriate for a "base" library like stactools to setup upper bounds. What if a downstream package requires a higher version of a package than what's allowed here?

And finally, IMO it's almost never appropriate for a library to declare an upper bound. Who's to say that stactools isn't compatible with the next release of that package? https://iscinumpy.dev/post/bound-version-constraints/ goes into this in much more detail, but the short version is that it's best for libraries to be as flexible as possible and let specific applications to pin their dependencies as they desire.

@TomAugspurger TomAugspurger added the bug Something isn't working label Feb 7, 2022
@gadomski
Copy link
Member

gadomski commented Feb 7, 2022

Would folks be OK with removing those upper bounds?

Yeah, I think we probably should relax -- @sgillies brought my attention to https://packaging.python.org/en/latest/discussions/install-requires-vs-requirements/ which seems to indicate we should relax any constraints that we don't need, and use a requirements.txt to specify exact dependency versions that are known to work. Happy to hear/learn more from others as well.

When I pinned all of those it was by instinct after coming from compiled languages (e.g. Rust) where this is generally best practice -- obviously things are different here.

@TomAugspurger
Copy link
Collaborator Author

👍 thanks.

One workflow question around testing: IME it's best to have 2-3 environments:

  1. An "earliest" / "oldest" environment with the minimum required dependencies for each library. This would match the lower bounds in the setup.cfg
  2. A "latest" / "newest" environment, that's either unpinned or pinned to the latest version + Dependabot to auto-upgrade when new packages are available. My preference is just for not pinning; We might have the occasional failure from an incompatible upgrade, but IMO that's OK.
  3. (maybe) An "upstream-dev" environment, that tests against unreleased versions of critical dependencies (maybe just pystac?).

Are folks OK with that rough that?

@gadomski
Copy link
Member

gadomski commented Feb 8, 2022

Sounds good to me, couple of questions/thoughts:

  • How do you go about specifying 1? Do you freeze a "minimum required" requirements.txt?
  • I think 3 should include rasterio as well as pystac as a --pre install.

@TomAugspurger
Copy link
Collaborator Author

How do you go about specifying 1? Do you freeze a "minimum required" requirements.txt?

Yeah, either a requirements file or pinned versions in the CI job that's building that environment.

@gadomski gadomski added this to the v0.3.0 milestone Feb 8, 2022
@gadomski gadomski mentioned this issue Feb 8, 2022
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants