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

[FEA] Reorganize Cython to separate C++ bindings and make Cython classes public #1280

Closed
vyasr opened this issue May 31, 2023 · 0 comments · Fixed by #1676
Closed

[FEA] Reorganize Cython to separate C++ bindings and make Cython classes public #1280

vyasr opened this issue May 31, 2023 · 0 comments · Fixed by #1676
Assignees
Labels
feature request New feature or request Python Related to RMM Python API

Comments

@vyasr
Copy link
Contributor

vyasr commented May 31, 2023

Is your feature request related to a problem? Please describe.
Currently rmm intermixes C++ API bindings and Cython declarations in the same pxd files. All of these pxd files are contained in the _lib module, whose name suggests that it is internal. However, this module is used by essentially all RAPIDS libraries because their Cython code compiling against Cython classes like rmm._lib.device_buffer.DeviceBuffer. Therefore, despite being ostensibly private, it is in fact treated as part of the public API by the rest of RAPIDS.

Describe the solution you'd like
The Cython C++ API bindings should be moved into a separate subpackage that is not conflated with the pure Cython code. The Cython code should be moved into a new subpackage whose name clearly indicates that it is intended to be public.

Describe alternatives you've considered
None

Additional context
Since any change here would be a breaking change to the rest of RAPIDS, it would be best to alias all existing types back into rmm._lib when the new module structure is created along with deprecation warnings. See #1221 for a template of how to do this.

@vyasr vyasr added feature request New feature or request ? - Needs Triage Need team to review and classify labels May 31, 2023
@harrism harrism added the Python Related to RMM Python API label May 31, 2023
@Matt711 Matt711 moved this from Todo to In Progress in RMM Project Board Sep 12, 2024
@wence- wence- removed the ? - Needs Triage Need team to review and classify label Sep 25, 2024
@rapids-bot rapids-bot bot closed this as completed in #1676 Oct 3, 2024
rapids-bot bot pushed a commit that referenced this issue Oct 3, 2024
…hon classes public (#1676)

Closes #1280

Authors:
  - Matthew Murray (https://github.com/Matt711)
  - Lawrence Mitchell (https://github.com/wence-)

Approvers:
  - Lawrence Mitchell (https://github.com/wence-)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #1676
@github-project-automation github-project-automation bot moved this from In Progress to Done in RMM Project Board Oct 3, 2024
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 Python Related to RMM Python API
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants