remove unused dependencies#25
Conversation
jameslamb
added
improvement
This comment was marked as resolved.
This comment was marked as resolved.
| - depends_on_rapids_logger | ||
| - depends_on_rmm | ||
| - develop | ||
| - docs |
There was a problem hiding this comment.
This project doesn't have any docs in source control. For now, let's remove the docs: group to avoid installing unnecessary packages.
It can be restored, hopefully with a more minimal set of dependencies focused on this repo, once there are docs to build.
| - depends_on_cupy | ||
| - depends_on_libcuforest | ||
| - depends_on_pylibraft | ||
| - depends_on_rmm |
There was a problem hiding this comment.
Nothing in this project uses the rmm Python package.
git grep rmm
| includes: | ||
| - cuda_wheels | ||
| - depends_on_libraft | ||
| - depends_on_librmm |
There was a problem hiding this comment.
There's a direct use of this here:
That's been working because libraft pulls in librmm transitively, but for completeness and protection against refactorings, every direct usage should be declared as a direct dependency.
| - joblib>=0.11 | ||
| - &numpy numpy>=1.23,<3.0a0 | ||
| - scipy>=1.8.0 | ||
| - packaging | ||
| - rich |
There was a problem hiding this comment.
None of these are directly imported in cuforest.
| - certifi | ||
| - *cython | ||
| - hdbscan>=0.8.39,<0.8.40 | ||
| - hypothesis>=6.0,<7 | ||
| - nltk | ||
| # upstream sklearn docstring tests require numpydoc<1.9 | ||
| - numpydoc<1.9 | ||
| # 'nvidia-ml-py' provides the 'pynvml' module | ||
| - nvidia-ml-py>=12 | ||
| - pyyaml | ||
| - pytest | ||
| - pytest-benchmark | ||
| - pytest-cases | ||
| - pytest-cov | ||
| - pytest-xdist | ||
| - seaborn | ||
| - *scikit_learn | ||
| - statsmodels | ||
| - tenacity | ||
| - umap-learn==0.5.7 | ||
| - pynndescent |
There was a problem hiding this comment.
None of these are needed for cuforest's tests. Seeing stuff like hdbscan and pynndescent makes me think this was probably just copied from cuML.
| # 'nvidia-ml-py' provides the 'pynvml' module | ||
| - nvidia-ml-py>=12 | ||
| - pyyaml | ||
| - pandas |
There was a problem hiding this comment.
Looks like there is some direct usage of pandas in tests.
ERROR python/cuforest/tests/test_cuforest.py - ImportError while importing test module '/__w/cuforest/cuforest/python/cuforest/tests/test_cuforest.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/opt/conda/envs/test/lib/python3.10/importlib/__init__.py:126: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
python/cuforest/tests/test_cuforest.py:10: in <module>
import pandas as pd
E ModuleNotFoundError: No module named 'pandas'
Here:
That was probably satisfied before by pandas being a dependency of one of these other test-time dependencies.
|
Thanks Gil! I'd like to give @csadorf a chance to review this before we merge. |
csadorf
left a comment
csadorf
left a comment
There was a problem hiding this comment.
Thanks a lot for working on this cleanup!
|
/merge |
3 participants
In a couple reviews here, I've noticed what look like unnecessary dependencies carried over from cuML's configuration.
This removes those.