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

Use scikit-build for the build process #976

Merged
merged 52 commits into from
Mar 11, 2022

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Feb 11, 2022

This PR replaces a lot of the setuptools logic with using scikit-build, which allows us to leverage the existing CMake configuration for the C++ components of RMM to build the Python package using standard pip- and setup.py-based installs. Replaces #637.

@vyasr vyasr added feature request New feature or request 3 - Ready for review Ready for review by team CMake non-breaking Non-breaking change labels Feb 11, 2022
@vyasr vyasr self-assigned this Feb 11, 2022
@vyasr vyasr requested review from a team as code owners February 11, 2022 01:10
@github-actions github-actions bot added the Python Related to RMM Python API label Feb 11, 2022
@vyasr
Copy link
Contributor Author

vyasr commented Feb 11, 2022

CC @kkraus14

Copy link
Contributor

@kkraus14 kkraus14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question, otherwise this looks amazing!

python/CMakeLists.txt Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor Author

vyasr commented Feb 14, 2022

rerun tests

@vyasr
Copy link
Contributor Author

vyasr commented Mar 8, 2022

Ah good catch, I had noticed that earlier but forgot to address it. I think we'll need to bring this up with the skbuild team, I think skbuild automatically identifies subpackage by __init__.py even if we don't tell it to compile anything there. I agree that this isn't a blocker though.

@vyasr
Copy link
Contributor Author

vyasr commented Mar 9, 2022

@ajschmidt8 and I just met and went through installing these conda packages into a fresh environment. Everything looks good, the package is well-formed and Python tests pass so at this point I think I'm comfortable proceeding. I think it's very unlikely that we will run into conda package issues at this point, and if someone finds bugs trying to build from source due to unexpected system configuration I expect them to be a small enough portion of the user base that we can work to resolve those issues if and when they arise.

@vyasr vyasr removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Mar 9, 2022
@vyasr
Copy link
Contributor Author

vyasr commented Mar 9, 2022

@robertmaynard @rapidsai/rmm-python-codeowners please take a final look at this PR, we can merge once you've approved.

build.sh Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
python/CMakeLists.txt Show resolved Hide resolved
python/CMakeLists.txt Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
build.sh Outdated Show resolved Hide resolved
Co-authored-by: Keith Kraus <keith.j.kraus@gmail.com>
@vyasr
Copy link
Contributor Author

vyasr commented Mar 11, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 36c13ff into rapidsai:branch-22.04 Mar 11, 2022
@vyasr vyasr deleted the scikit_build branch February 18, 2023 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for review Ready for review by team CMake conda feature request New feature or request gpuCI non-breaking Non-breaking change Python Related to RMM Python API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants