Add missing nvcomp targets #496
Merged
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
vyasr
added
improvement
robertmaynard
approved these changes
Dec 4, 2023
robertmaynard
requested changes
Dec 4, 2023
robertmaynard
approved these changes
Dec 4, 2023
|
/merge |
3 tasks
raydouglass
pushed a commit
to rapidsai/cudf
that referenced
this pull request
Dec 11, 2023
This PR makes a number of changes to streamline the CMake code in the Python build. There are additional potential improvements that may be possible but I'd prefer to wait on until after the scikit-build-core migration because I suspect that there may be subtle differences (particularly around the install rules). The major improvements here are: - nvcomp installation no longer needs as much special casing thanks to rapidsai/rapids-cmake#496 adding missing targets - get_arrow.cmake now determines how to find arrow by checking for 1) the presence of Python, 2) the presence of pyarrow, and 3) the presence of libarrow.so.* inside the pyarrow directory. This approach works for both pip and conda packages as well as pure C++ builds and removes the need for manual determination of where to look by the builder. The assumption baked in is that the developer has installed the desired pyarrow package when building. - The `CUDF_BUILD_WHEELS` variable is now removed. All Python builds that don't find the C++ library all go down the same path now. This is particularly relevant for doing something like a local `pip install`. This is the desired behavior because if you're building the Python package in that kind of environment you always want the same behaviors. The different case is when the Python build finds the C++ build (e.g. in a conda environment where libcudf is a separate package). - The logic for linking to pyarrow headers is now centralized in a single function since it needs to be used by every Cython target. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - https://github.com/brandon-b-miller - Robert Maynard (https://github.com/robertmaynard) - Ray Douglass (https://github.com/raydouglass)
karthikeyann
pushed a commit
to karthikeyann/cudf
that referenced
this pull request
Dec 12, 2023
This PR makes a number of changes to streamline the CMake code in the Python build. There are additional potential improvements that may be possible but I'd prefer to wait on until after the scikit-build-core migration because I suspect that there may be subtle differences (particularly around the install rules). The major improvements here are: - nvcomp installation no longer needs as much special casing thanks to rapidsai/rapids-cmake#496 adding missing targets - get_arrow.cmake now determines how to find arrow by checking for 1) the presence of Python, 2) the presence of pyarrow, and 3) the presence of libarrow.so.* inside the pyarrow directory. This approach works for both pip and conda packages as well as pure C++ builds and removes the need for manual determination of where to look by the builder. The assumption baked in is that the developer has installed the desired pyarrow package when building. - The `CUDF_BUILD_WHEELS` variable is now removed. All Python builds that don't find the C++ library all go down the same path now. This is particularly relevant for doing something like a local `pip install`. This is the desired behavior because if you're building the Python package in that kind of environment you always want the same behaviors. The different case is when the Python build finds the C++ build (e.g. in a conda environment where libcudf is a separate package). - The logic for linking to pyarrow headers is now centralized in a single function since it needs to be used by every Cython target. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - https://github.com/brandon-b-miller - Robert Maynard (https://github.com/robertmaynard) - Ray Douglass (https://github.com/raydouglass)
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.
Description
The list of targets marked as global when finding nvcomp is incomplete and should include the gdeflate and bitcomp libs as well.
Checklist
cmake-format.jsonis up to date with these changes.include_guard(GLOBAL))