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 literals have correct dtype #15890

Merged
merged 3 commits into from
Jun 4, 2024
Merged

Ensure literals have correct dtype #15890

merged 3 commits into from
Jun 4, 2024

Conversation

wence-
Copy link
Contributor

@wence- wence- commented May 30, 2024

Description

The polars schema tells us the dtype for any literals, but previously we were relying on pyarrow inference. Add pylibcudf to pyarrow datatype conversion utilities and use the resulting datatypes explicitly.

Checklist

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

@wence- wence- requested a review from a team as a code owner May 30, 2024 17:07
@wence- wence- requested review from vyasr and isVoid May 30, 2024 17:07
@github-actions github-actions bot added the Python Affects Python cuDF API. label May 30, 2024
@wence- wence- added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change cudf.polars Issues specific to cudf.polars and removed Python Affects Python cuDF API. labels May 30, 2024
lithomas1
lithomas1 reviewed May 30, 2024
View reviewed changes
Copy link
Contributor

@lithomas1 lithomas1 left a comment

Choose a reason for hiding this comment

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

This LGTM, but I'll let someone else approve since I'm unfamiliar with polars code so far.

I left a couple nits/questions.

python/cudf_polars/cudf_polars/dsl/expr.py Outdated Show resolved Hide resolved
python/cudf_polars/cudf_polars/utils/dtypes.py Outdated Show resolved Hide resolved
python/cudf_polars/tests/test_scan.py Show resolved Hide resolved
@wence- wence- force-pushed the wence/fea/cudf-polars-literals branch from c12a5b4 to b8bd4a3 Compare May 31, 2024 10:57
@wence- wence- requested a review from a team as a code owner May 31, 2024 10:57
@github-actions github-actions bot added the Python Affects Python cuDF API. label May 31, 2024
@wence- wence- force-pushed the wence/fea/cudf-polars-literals branch from b8bd4a3 to 3a2c14e Compare June 4, 2024 12:26
@wence-
Copy link
Contributor Author

wence- commented Jun 4, 2024

Ready for another look, @brandon-b-miller and @lithomas1

brandon-b-miller
brandon-b-miller approved these changes Jun 4, 2024
View reviewed changes
Copy link
Contributor

@brandon-b-miller brandon-b-miller left a comment

Choose a reason for hiding this comment

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

one comment, else LGTM

@wence- wence- force-pushed the wence/fea/cudf-polars-literals branch from 3a2c14e to 9bf53c3 Compare June 4, 2024 13:26
@wence-
Copy link
Contributor Author

wence- commented Jun 4, 2024

/merge

@wence- wence- added the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jun 4, 2024
@brandon-b-miller brandon-b-miller mentioned this pull request Jun 4, 2024
Merged
@github-actions github-actions bot added the pylibcudf Issues specific to the pylibcudf package label Jun 4, 2024
@wence- wence- removed the 5 - DO NOT MERGE Hold off on merging; see PR for details label Jun 4, 2024
@wence-
Copy link
Contributor Author

wence- commented Jun 4, 2024 •
edited
Loading

Minor fixup to the pylibcudf datatype interop changes was needed as well.

@rapids-bot rapids-bot bot merged commit 54d49fc into rapidsai:branch-24.08 Jun 4, 2024
70 checks passed
@wence- wence- deleted the wence/fea/cudf-polars-literals branch June 4, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lithomas1 lithomas1 lithomas1 left review comments

@brandon-b-miller brandon-b-miller brandon-b-miller approved these changes

@jameslamb jameslamb jameslamb approved these changes

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

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

Assignees
No one assigned
Labels
cudf.polars Issues specific to cudf.polars 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
cuDF Python
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wence- @jameslamb @lithomas1 @brandon-b-miller