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.

Sign up for GitHub

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

Jump to bottom

Ensure Cython bindings only interact with pylibcudf objects #1523

Merged
merged 5 commits into from
Feb 3, 2025
Merged

Ensure Cython bindings only interact with pylibcudf objects #1523

merged 5 commits into from
Feb 3, 2025

Conversation

mroeschke
Copy link
Contributor

@mroeschke mroeschke commented Jan 30, 2025 •
edited
Loading

Description

Follow up to #1514

Addressing the review in #1514 (comment) by ensuring the Cython bindings return pylibcudf objects where possible instead of python objects (like cudf Python columns) so we can also have better return annotations for these methods where appropriate. Subsequently, the conversion from pylibcudf to cudf Python has been moved to the cuspatial Python layer.

Additionally, this PR will be necessary for rapidsai/cudf#17760 not to break cuspatial

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 30, 2025
@mroeschke mroeschke self-assigned this Jan 30, 2025
@mroeschke mroeschke requested a review from a team as a code owner January 30, 2025 22:33
@mroeschke mroeschke requested review from trxcllnt and harrism January 30, 2025 22:33
@isVoid
Copy link
Contributor

isVoid commented Jan 31, 2025 •
edited
Loading

Why are the tests skipped?
https://github.com/rapidsai/cuspatial/actions/runs/13062519158/job/36450294299?pr=1523
https://github.com/rapidsai/cuspatial/actions/runs/13062519158/job/36449987606?pr=1523

@bdice bdice mentioned this pull request Jan 31, 2025
Merged
3 tasks
rapids-bot bot pushed a commit that referenced this pull request Feb 1, 2025
@bdice
Use test_python changed files list for Python tests. (#1527)
536075f 
Fixes undesired CI skips in #1523.

Authors:
  - Bradley Dice (https://github.com/bdice)
  - Matthew Roeschke (https://github.com/mroeschke)

Approvers:
  - James Lamb (https://github.com/jameslamb)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Jake Awe (https://github.com/AyodeAwe)

URL: #1527
bdice
bdice approved these changes Feb 3, 2025
View reviewed changes
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.

One minor whitespace fix, which I can apply. Approving.

python/cuspatial/cuspatial/_lib/nearest_points.pyx Outdated Show resolved Hide resolved
@mroeschke
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 57a34d7 into rapidsai:branch-25.02 Feb 3, 2025
74 checks passed
@mroeschke mroeschke deleted the ref/cython/plc_only branch February 3, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isVoid isVoid isVoid approved these changes

@bdice bdice bdice approved these changes

@trxcllnt trxcllnt Awaiting requested review from trxcllnt trxcllnt is a code owner automatically assigned from rapidsai/cuspatial-python-codeowners

@harrism harrism Awaiting requested review from harrism harrism is a code owner automatically assigned from rapidsai/cuspatial-python-codeowners

Assignees

@mroeschke mroeschke

Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Related to Python code
Projects
cuSpatial
Status: Todo
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mroeschke @isVoid @bdice