[WIP] Make dask an optional dependency
#6240
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This moves all dask dependencies of `cuml` to be optional. `pip` users can install them via a `dask` extra (e.g. `pip install cuml[dask]`), while conda users can install them with a conda metapackage (e.g. `pip install cuml-dask`). Dask dependency versions are pinned accordingly as needed.
We now only install dask dependencies for the dask test runs.
- Removes dask imports from everywhere except the `cuml.dask` namespace - Cleans up the imports in the `cuml.dask` namespace. In particular, non of the existing warning code was actually functional. - Adds a nice `ImportError` for users trying to import `cuml.dask` without the required dependencies.
github-actions
bot
added
conda
jcrist
commented
Jan 21, 2025
There was a problem hiding this comment.
There's still some failing tests in cuml/tests/ that rely on dask being present:
test_base/test_pickle: these create shared tests for all estimators, but obviously the dask tests can't run when dask isn't installed. Easiest fix would be to skip those automatically if dask isn't installed, and add these files to the dask test runs in CI so they still run somewhere.test_benchmark: some benchmarks rely on dask. Could skip these if dask isn't installed?test_serialize.py:test_naive_bayes_cuda: this is a dask specific test, but afaict we're just testing that dask's serializers can serialize a cuml class? This test should probably be moved into thetests/daskfolder.
| - cuda-cudart-dev | ||
| {% endif %} | ||
| - cuda-python | ||
| outputs: |
There was a problem hiding this comment.
I haven't verified this recipe works at all, just based it on other recipes I've done in the past that have used multiple outputs for metapackages. Hoping CI/review will catch any issues here 🤞 .
| " conda install cuml-dask # either conda install\n" | ||
| ' pip install -U "cuml[dask]" # or pip install\n' | ||
| ) | ||
| raise ImportError(msg) from exc |
There was a problem hiding this comment.
Here we raise a nicer ImportError when a user tries to import something from cuml.dask when all dependencies aren't installed.
| else: | ||
| warnings.warn( | ||
| "Dask not found. All Dask-based multi-GPU operation is disabled." | ||
| ) |
There was a problem hiding this comment.
This previous warnings code would never run. If a dep wasn't present, the error would occur far before here. Better to just use normal imports.
jcrist
added
Build or Dep
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes
dask/distributed/dask_cudf/raft_dask/dask_cudaoptional dependencies.By default, an install of
cumlwill not include these dependencies. Users will not be able to use the functionality available incuml.dask, but everything else will work as is.To install
cumlwithdasksupport, users can pip or conda install the extra deps asFixes #5934.