-
Notifications
You must be signed in to change notification settings - Fork 197
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
Allow testing of earliest/latest dependencies. #1606
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -306,3 +306,14 @@ dependencies: | |
- cuda-nvcc | ||
- matrix: | ||
packages: | ||
specific: | ||
- output_types: [conda, requirements, pyproject] | ||
matrices: | ||
- matrix: | ||
dependencies: "earliest" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This worked as desired:
...
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
make sense? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have an opinion on naming, the name "earliest" already exists in the common workflow (but not sure it is actually used for much there). earliest/latest is only really relevant for testing, so I think we don't need any of the other things mentioned and this is just fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed a bit to realize that a currently valid way to specify say that something works for say Python 3.12 and 3.13 is Likely not often needed in rapids, but it occurred to me that minimal versions can often be Python version dependent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In RAPIDS we have historically chosen lower bounds that work for all RAPIDS-supported Python versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I chose “earliest/latest” because we named our CUDA drivers that way in shared-workflows. There is also precedent for “oldest” in packages like “oldest-supported-numpy.” Please feel free to change it — I have bequeathed these pull requests to @seberg. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have a very weak preference to avoid calling it “minimum” versions because we usually say a package with few dependencies or no optional dependencies has “minimal dependencies.” |
||
packages: | ||
- numba==0.57.* | ||
- numpy==1.23.* | ||
# No pinnings except for earliest dependencies | ||
- matrix: | ||
packages: |
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.
For wheel tests, we will have to engineer a way to use the oldest dependencies. We determine the test dependencies at build time, via
[test]
extras that are baked into the package. However, for testing earliest dependency pinnings, we really just want to add some extra pinnings to the environment at test time based on the value ofRAPIDS_DEPENDENCIES
. We may have to add a call inci/test_wheel.sh
that invokesrapids-dependency-file-generator
to create arequirements.txt
file that we install after installing the previously-built wheel.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.
Nice, that makes sense to me (and it is really cool to see how straight forward this is looking like).
Don't want to mess with the builds, just testing, so branching between using
package[test]
and a custom requirements install onRAPIDS_DEPENDENCIES="earliest"
seems good.