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

Add pylibcudf.null_mask.null_count #17711

Merged
merged 7 commits into from
Jan 14, 2025

Conversation

mroeschke
Copy link
Contributor

Description

A small step to not have null_count tied to 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 improvement Improvement / enhancement to an existing function non-breaking Non-breaking change pylibcudf Issues specific to the pylibcudf package labels Jan 10, 2025
@mroeschke mroeschke self-assigned this Jan 10, 2025
@mroeschke mroeschke requested a review from a team as a code owner January 10, 2025 00:40
@mroeschke mroeschke requested review from vyasr and Matt711 January 10, 2025 00:40
@github-actions github-actions bot added the Python Affects Python cuDF API. label Jan 10, 2025
Copy link
Contributor

@Matt711 Matt711 left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me. I had one clarifying question and doc string suggestion (both non-blocking).

python/pylibcudf/pylibcudf/null_mask.pyx Outdated Show resolved Hide resolved
@@ -148,3 +150,28 @@ cpdef tuple bitmask_or(list columns):
c_result = cpp_null_mask.bitmask_or(c_table.view())

return buffer_to_python(move(c_result.first)), c_result.second


cpdef size_type null_count(Py_ssize_t bitmask, size_type start, size_type stop):
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you using Py_ssize_t over int because the bitmask is essentially an index to a python buffer object? And according to this stackoverflow post. Py_ssize_t preffered over int when indexing into python objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was matching the ptr type of int_to_void_ptr here. Additionally, using int here would result in an OverflowError

@mroeschke
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit e0dac5d into rapidsai:branch-25.02 Jan 14, 2025
106 checks passed
@mroeschke mroeschke deleted the plc/null_count branch January 14, 2025 23:06
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 pylibcudf Issues specific to the pylibcudf package Python Affects Python cuDF API.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants