-
Notifications
You must be signed in to change notification settings - Fork 197
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
remove unused 'joblib' and 'numba' dependencies, other packaging cleanup #2532
remove unused 'joblib' and 'numba' dependencies, other packaging cleanup #2532
Conversation
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
/ok to test |
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.
LGTM! It seems like setup.cfg
is mostly duplicating the settings we already have in pyproject.toml
, so I assume this was an oversight and not removed earlier. Either way the final state looks good to me.
Came here looking to remove the `setup.cfg` file, inspired by rapidsai/raft#2532. But this PR uncovered a few other packaging things to be cleaned up. Proposes: * removing `Cython` dependency * aligning `cmake_minimum_required()` version with the one from the conda recipe, and with what RAPIDS has been using for the last few releases * removing `maptlolib` dependency for benchmarks ## Notes for Reviewers ### How is it safe to remove `Cython`? As far as I can tell, this project doesn't have any Cython code. It is building a shared library and then loading it with `ctypes`: https://github.com/rapidsai/legate-raft/blob/3260952df5cc09fe356ccf07e8832070f1029abb/legate_raft/library.py#L21-L26 Signed-off-by: James Lamb <[email protected]>
Similar to rapidsai/raft#2532, this proposes some small packaging cleanup. * removes `setup.cfg` files - *these are currently being ignored by tools, in favor of identical configuration in `pyproject.toml` and `.flake8` files* - e.g. https://github.com/rapidsai/cuvs/blob/b3ce774d39e149d4e34c401068f24136eac44e13/.pre-commit-config.yaml#L31-L35 * alphabetizes dependency lists in `dependencies.yaml` * changes `cupy:` group in `dependencies.yaml` to `depends_on_cupy:` (for consistency with other dependencies) Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) - Micka (https://github.com/lowener) URL: #544
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.
LGTM no concerns from my end.
/merge |
Proposes some cleanup of packaging details, noticed while I was working on #2531
joblib
andnumba
forraft-dask
raft-dask
doesn't directly import from these libraries, and the git blame didn't suggest any other reason that they were being pinned heregit grep -E 'joblib|numba'
setup.cfg
filespyproject.toml
and.flake8
filesraft/.pre-commit-config.yaml
Lines 16 to 19 in bfd1906