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

Make CategoricalColumn.__init__ strict #16456

Merged
merged 4 commits into from
Aug 16, 2024

Conversation

mroeschke
Copy link
Contributor

@mroeschke mroeschke commented Aug 1, 2024

Description

This PR transfers some of the validation logic in build_column directly into CategoricalColumn just in case CategoricalColumn is called independently of build_column. Additionally adds stricter validation of data, dtype and children so the column doesn't represent an invalid state

xref #16469

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 Affects Python cuDF API. improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 1, 2024
@mroeschke mroeschke requested a review from a team as a code owner August 1, 2024 00:08
and isinstance(children[0], cudf.core.column.numerical.NumericalColumn)
and children[0].dtype.kind in "iu"
):
# TODO: Enforce unsigned integer?
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it need to be? This would be removing the i in quotes above, I assume.

Copy link
Contributor Author

@mroeschke mroeschke Aug 16, 2024

Choose a reason for hiding this comment

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

I guess it doesn't strictly need to be since libcudf will probably do that casting if needed. And yeah we would just remove the i

@mroeschke
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit fd44adc into rapidsai:branch-24.10 Aug 16, 2024
80 checks passed
@mroeschke mroeschke deleted the ref/catcol/strict branch August 16, 2024 23:02
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 Affects Python cuDF API.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants