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

WIP: install 'hdbscan' from PyPI #5935

Closed
wants to merge 1 commit into from

Conversation

jameslamb
Copy link
Member

Description

Proposes:

  • getting test dependency hdbscan from PyPI instead of GitHub
  • ensuring rapids-dependency-file-generator --output requirements generates a full set of test dependencies

Benefits of these changes

  • ensures devcontainers environment includes all of cuml's test dependencies
  • removes a dependency on building from the tip of a remote git repo, and all the potential for instability and security issues that comes with that

Notes for Reviewers

I tried testing a devcontainers change today (rapidsai/devcontainers#325), like this:

# clean previous build outputs
uninstall-all -j
clean-all -j

# work on latest branches of everything
rapids-checkout-same-branch --omit ucxx
git -C ./ucxx checkout branch-0.39
rapids-pull-repositories

# install all the other dependencies
rapids-make-pip-env --force

# build all the packages
build-all -j

# run the cuml unit tests
cd ./cuml
./ci/run_cuml_singlegpu_pytests.sh

Found that the cuml unit tests failed immediately with an error complaining about hdbscan not being available. Looks like that's because it's used unconditionally here:

This hasn't shown up in CI because in CI, the cuml wheel's [test] extra is used to figure out what dependencies to install:

python -m pip install $(echo ./dist/cuml*.whl)[test]

But RAPIDS devcontainers use rapids-dependency-file-generator --output requirements:

https://github.com/rapidsai/devcontainers/blob/0ec3cbd47af5ffdc1399af2cb9e33e34617230c3/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-pip-dependencies.sh#L90-L96

@jameslamb jameslamb added 2 - In Progress Currenty a work in progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 14, 2024
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Jun 14, 2024
@dantegd
Copy link
Member

dantegd commented Jun 14, 2024

Thanks @jameslamb, @divyegala was working on this in this PR #5906. The issue we ran into is that hdbscan requires cython >=0.27,<3 with the potential options at runtime, which is the same issue this PR has already ran into in the pip devcontainer job

@jameslamb
Copy link
Member Author

🤦🏻 I should have checked the open PRs, was a little too quick here. Thanks @dantegd and sorry for the noise, I'll close this.

@jameslamb jameslamb closed this Jun 14, 2024
@jameslamb jameslamb deleted the hdbscan-dep branch June 14, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - In Progress Currenty a work in progress Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants