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

Support scikit-build-core #433

Merged
merged 31 commits into from
Dec 12, 2023

Conversation

vyasr
Copy link
Contributor

@vyasr vyasr commented Jun 29, 2023

Description

This PR adds a new cython-core module of rapids-cmake that can be used with scikit-build-core. cython-core is designed as a drop-in replacement for cython, allowing consumers to simply change the include(cython) to include(cython-core) and have things generally work as expected when switching Python build backends.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@vyasr vyasr self-assigned this Jun 29, 2023
@vyasr vyasr added feature request New feature or request non-breaking Introduces a non-breaking change labels Jun 29, 2023
Copy link

copy-pr-bot bot commented Dec 1, 2023

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@vyasr vyasr changed the base branch from branch-23.08 to branch-24.02 December 1, 2023 23:31
@vyasr vyasr marked this pull request as ready for review December 7, 2023 03:21
@vyasr vyasr requested a review from a team as a code owner December 7, 2023 03:21
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Are cython and cython-core meant to be used separately? Do we need to document that or raise an error if a project uses both? Can projects mix cython and cython-core in dependencies, like rmm using cython-core while cudf uses cython?

rapids-cmake/cython-core/create_modules.cmake Outdated Show resolved Hide resolved
rapids-cmake/cython-core/create_modules.cmake Outdated Show resolved Hide resolved
rapids-cmake/cython-core/create_modules.cmake Show resolved Hide resolved
rapids-cmake/cython-core/create_modules.cmake Outdated Show resolved Hide resolved
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

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

Missing documentation and cmake-format updates

@vyasr
Copy link
Contributor Author

vyasr commented Dec 12, 2023

Missing documentation and cmake-format updates

I added docs. The new functions are identical to the old ones, just included differently (include(rapids-cython-core) instead of include(rapids-cython)), so there are no new additions required for cmake-format at this stage.

rapids-cmake/cython/init.cmake Show resolved Hide resolved
@vyasr
Copy link
Contributor Author

vyasr commented Dec 12, 2023

/merge

@rapids-bot rapids-bot bot merged commit 287a970 into rapidsai:branch-24.02 Dec 12, 2023
17 checks passed
@vyasr vyasr deleted the feat/scikit-build-core branch December 14, 2023 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants