-
Notifications
You must be signed in to change notification settings - Fork 908
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
Build wheels alongside conda CI #12427
Build wheels alongside conda CI #12427
Conversation
Codecov ReportBase: 86.58% // Head: 85.67% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## branch-23.02 #12427 +/- ##
================================================
- Coverage 86.58% 85.67% -0.91%
================================================
Files 155 155
Lines 24368 24803 +435
================================================
+ Hits 21098 21251 +153
- Misses 3270 3552 +282
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
(double approval because I forgot the 🔥 in the first one)
🔥 🔥 🔥 🔥 🔥 🔥
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.
Couple of minor suggestions but looks awesome!
package-name: cudf | ||
package-dir: python/cudf | ||
versioneer-override: ${{ inputs.wheel_versioneer_override }} | ||
skbuild-configure-options: "-DCUDF_BUILD_WHEELS=ON -DDETECT_CONDA_ENV=OFF" |
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.
We can leave this for now (something I will raise more broadly in the new year) but AFAIK this variable doesn't do anything right now. The question would be whether it should be set to do anything. That's a rapids-wide discussion though; I think only cuML is actively making use of this now and that's partly because it needs to forward along to deps like treelite.
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.
with: | ||
build_type: pull-request | ||
package-name: cudf | ||
test-before-arm64: "pip install tokenizers==0.10.2 cupy-cuda11x -f https://pip.cupy.dev/aarch64" |
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.
Since it's already working we can leave it for now, but I'll note that in #12377 we actually removed tokenizers from arm tests and only test on x86 for conda packages.
Create new wheel build parameters: `before-wheel` This allows us to use `rapids-download-wheels-from-s3` to have inter-dependent wheels within a single workflow download the dependency wheels _built in this current workflow run_ instead of pulling the nightly published copy. Tested for pure wheels in cuDF: rapidsai/cudf#12427 Tested for manylinux wheels in RAFT: rapidsai/raft#1116
/merge |
Description
This PR adds pip wheel CI to the Conda CI, instead of having them work separately.
Checklist