-
Notifications
You must be signed in to change notification settings - Fork 21
rapids-generate-pip-constraints: allow any value for RAPIDS_DEPENDENCIES #239
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
Conversation
|
|
||
| if [[ "${RAPIDS_DEPENDENCIES}" == "oldest" ]]; then | ||
| if [[ "${RAPIDS_DEPENDENCIES}" == "oldest" ]] \ | ||
| || [[ "${RAPIDS_DEPENDENCIES}" == "intermediate" ]] \ |
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.
If there were 5 or 10 known values that were desired here maybe we should do something more general, but I don't think there are right now. Just adding one more condition here seems fine to me.
csadorf
left a comment
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.
Actually, we introduced yet another custom dependency group in rapidsai/cuml#7684: "nightly"
My draft implementation was wrong when I made this comment.
|
Ah no worries, I've pushed 92a6947 adding |
csadorf
left a comment
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.
Based on https://github.com/rapidsai/cuml/actions/runs/21074900945/job/60622756913?pr=7684#step:13:105; this appears to be working.
vyasr
left a comment
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.
Rather than having specific supported versions, can we just make latest a no-op and transparently forward anything else through? The dfg call will fail loudly anyway if provided a file key that doesn't exist.
|
I did consider that and tried to anticipate a comment like that with this thread: #239 (comment)
But sure, I don't feel that strongly, let's make it more general. Here's the commit doing that: b809958 |
vyasr
left a comment
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.
Sorry I missed your previous comment! In any case the improvement seems worthwhile and should save us from having to change anything else going forward.
|
@jameslamb You might want to update the PR description. |
|
Caught me right before I merged haha. I've updated the title and description. |
|
@csadorf this will take effect as soon as https://github.com/rapidsai/ci-imgs/actions/runs/21082919779 completes |
See https://github.com/rapidsai/cuml/pull/7684/files#r2695123359
rapids-generate-pip-constraintsis use to generate aconstraints.txtfile to accomplish stuff like "test against oldest versions of dependencies".The supported values are:
cuML defines 2 more:
scikit-learnversions they support)This extends
rapids-generate-pip-constraintsto allow any value forRAPIDS_DEPENDENCIES. It also updatespre-commithooks here to their latest versions, since we're touching the repo anyway.