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

Have Cython methods accept pylibcudf.Column instead of cudf._lib.column.Column #1514

Open
wants to merge 2 commits into
base: branch-25.02
Choose a base branch
from

Conversation

mroeschke
Copy link
Contributor

Description

Related to rapidsai/cudf#17317, it appears the Cython bindings are not dependent on any methods specifically on cudf._lib.column.Column. It appears most routines operate on column_view which can be provided by a pylibcudf.Column.

This PR essentially makes the routines defined in Cython accept a pylibcudf.Column instead and return a cudf.core.column.Column object instead (which should help cudf transition away from its cudf._lib.column.Column)

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@mroeschke mroeschke added Python Related to Python code improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 14, 2025
@mroeschke mroeschke requested a review from a team as a code owner January 14, 2025 02:14
@mroeschke mroeschke requested review from thomcom and bdice January 14, 2025 02:14
@@ -26,7 +26,12 @@ from cuspatial._lib.cpp.types cimport collection_type_id, geometry_type_id
from cuspatial._lib.types cimport collection_type_py_to_c


cpdef haversine_distance(Column x1, Column y1, Column x2, Column y2):
cpdef haversine_distance(
plc_Column x1,
Copy link
Member

Choose a reason for hiding this comment

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

Can these just be imported as Column?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll have a namespace shadowing issue because we need cudf.core.column.Column (for the output) and pylibcudf.Column (to type these inputs). I decided to alias pylibcudf.Column to plc_Column, but happy to use another alias if you'd like

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Related to Python code
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants