DEP: add missing lower bounds to dependencies from oldestdeps-alldeps#16960
DEP: add missing lower bounds to dependencies from oldestdeps-alldeps#16960pllim merged 1 commit intoastropy:mainfrom
oldestdeps-alldeps#16960Conversation
|
Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.
|
|
👋 Thank you for your draft pull request! Do you know that you can use |
| "asdf-astropy>=0.3", | ||
| "bottleneck", | ||
| "certifi>=2022.6.15.1", | ||
| "dask[array]>=2022.5.1", |
There was a problem hiding this comment.
For dask I was able to actually test this lower bound (the previous one, 2022.8.1, was just a guess I took in #16903)
c3b2692 to
f77e9d6
Compare
pyproject.toml
Outdated
| "pytest>=7.3.0", # 2023-04-08 | ||
| "pytest-doctestplus>=0.12", # 2022-02-25 | ||
| "pytest-astropy-header>=0.2.1", # 2022-03-10 | ||
| "pytest-astropy>=0.10.0", # 2022-04-12 |
There was a problem hiding this comment.
I wonder if some of the pytest- pins are outdated. Most of them should have come from pytest-astropy meta package.
There was a problem hiding this comment.
this is actually more restrictive than what's specified in pytest-astropy 0.10.0.
Furthermore, --resolution=lowest-direct doesn't take the lower bound of indirect dependencies: we should still specify some lower bound for ourselves if we really mean to check compatibility with anything older than the most recent version of these packages.
https://github.com/astropy/pytest-astropy/blob/v0.10.0/setup.cfg
There was a problem hiding this comment.
doesn't take the lower bound of indirect dependencies
Tsk tsk. Doesn't this defeats the purpose of our pytest-astropy meta-package pins then?
There was a problem hiding this comment.
Well, uv also supports --resolution=lowest, which is probably closer to what you expect here, but it's much less likely to work for us given how large our fully resolved dependency tree, so, I'm going for the strategy with the highest chance of success first. We can maybe explore this option later on if you'd like, but I must say I am pessimistic about its odds.
eerovaher
left a comment
There was a problem hiding this comment.
Writing down the minimum supported versions for all the dependencies is a good thing, but I don't see what's the benefit of writing down the release dates of all these versions.
pyproject.toml
Outdated
| "dask[array]>=2022.5.1", | ||
| "h5py>=3.8.0", # 2023-01-23 | ||
| "pyarrow>=10.0.1", # 2022-11-22 | ||
| "beautifulsoup4>=4.9.3", # 2020-10-03, imposed by pandas==2.0 |
There was a problem hiding this comment.
It is good to record that this version requirement is imposed by another package, much more valuable than the release date.
I often find myself wondering if some dependencies are old enough that maybe I can bump them without feeling guilty, so this helps with that. |
|
Another gain I discovered while migrating a bunch of projects' CI to uv is that having release dates right here makes it much easier if we need to set |
|
Please let me know if the inclusion of release dates is a show stopper for this PR and I'll remove them, they're not essential. |
|
The release dates add clutter to our configuration, they bloat the patch and they create a needless implicit expectation that the version release dates must be updated together with the dependencies themselves.
The date in the calendar is not a valid reason for updating our dependency requirements. Valid reasons are either security related, in which case release dates don't matter, or the removal or avoidance of version-dependent code in |
f77e9d6 to
d2dfb41
Compare
|
removed |
eerovaher
left a comment
There was a problem hiding this comment.
It is good to have explicit and tested version requirements for all our dependencies.
|
Thanks, all! |
Description
While working on #16950, I found that the proposed solution to use
uv pip install --resolution=lowest-directinoldestdepsonly really worked for dependencies with an explicit lower bound, so, after a lot of trial and errors, here's all the missing metadata I was able to discover. I think it has value beyond the scope of #16950 so I'm proposing it as its own PR. It actually end up being most of the change needed to implement "oldestdepswith uv" (the rest of the work will happen overtox.ini).I also threw in the release date for each of these dependencies that doesn't use calendar-based-versionning.