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

Bump Polars version to <1.18 #17632

Merged
merged 19 commits into from
Jan 9, 2025
Merged

Bump Polars version to <1.18 #17632

merged 19 commits into from
Jan 9, 2025

Conversation

Matt711
Copy link
Contributor

@Matt711 Matt711 commented Dec 19, 2024 •
edited
Loading

Description

This PR upgrades the Polars version to 1.17. It xfails some polars tests due to known issues and adds the maintain_order param to joins (not implemented yet).

Notable change

Checklist

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

@Matt711 Matt711 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Dec 19, 2024
@Matt711 Matt711 self-assigned this Dec 19, 2024
@copy-pr-bot copy-pr-bot
Copy link

copy-pr-bot bot commented Dec 19, 2024

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@github-actions github-actions bot added Python Affects Python cuDF API. cudf.polars Issues specific to cudf.polars labels Dec 19, 2024
@Matt711
Copy link
Contributor Author

Matt711 commented Dec 19, 2024

/ok to test

@Matt711 Matt711 marked this pull request as ready for review December 19, 2024 23:39
@Matt711 Matt711 requested review from a team as code owners December 19, 2024 23:39
@Matt711 Matt711 changed the title [WIP] Upgrade Polars version to 1.17 Upgrade Polars version to 1.17 Dec 19, 2024
wence-
wence- reviewed Dec 20, 2024
View reviewed changes
dependencies.yaml Outdated Show resolved Hide resolved
Matt711
Matt711 commented Dec 20, 2024
View reviewed changes
conda/environments/all_cuda-118_arch-x86_64.yaml Outdated Show resolved Hide resolved
conda/environments/all_cuda-125_arch-x86_64.yaml Outdated Show resolved Hide resolved
conda/recipes/cudf-polars/meta.yaml Outdated Show resolved Hide resolved
dependencies.yaml Outdated Show resolved Hide resolved
python/cudf_polars/pyproject.toml Outdated Show resolved Hide resolved
@vyasr
Copy link
Contributor

vyasr commented Dec 20, 2024

It looks like we also need an IR version bump.

@Matt711 Matt711 changed the title Upgrade Polars version to 1.17 Upgrade Polars version to 1.18 Dec 20, 2024
@Matt711 Matt711 requested review from a team as code owners January 2, 2025 15:48
@github-actions github-actions bot added libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas pylibcudf Issues specific to the pylibcudf package labels Jan 2, 2025
@github-actions github-actions bot removed libcudf Affects libcudf (C++/CUDA) code. CMake CMake build issue Java Affects Java cuDF API. cudf.pandas Issues specific to cudf.pandas pylibcudf Issues specific to the pylibcudf package labels Jan 8, 2025
@Matt711 Matt711 mentioned this pull request Jan 8, 2025
Open
mroeschke
mroeschke reviewed Jan 8, 2025
View reviewed changes
python/cudf_polars/cudf_polars/dsl/ir.py
@@ -1019,7 +1021,27 @@ class ConditionalJoin(IR):
__slots__ = ("ast_predicate", "options", "predicate")
_non_child = ("schema", "predicate", "options")
predicate: expr.Expr
options: tuple
"""Expression predicate to join on"""
Copy link
Contributor

Choose a reason for hiding this comment

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

In a follow up, could you put these docstrings in the main docstring of the class instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will do. And I think a lot in cudf.polars classes need improved docstrings. I guess we should just fix them if we make changes to them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably @wence- had a reason for doing it this way. Perhaps pyright or other LSPs preferred this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay cool will wait to see what Lawrence thinks

@Matt711
Copy link
Contributor Author

Matt711 commented Jan 9, 2025

/merge

bdice
bdice approved these changes Jan 9, 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.

Approving packaging changes. I think my approval will automerge this since a /merge was already given. I see there is an open discussion but I don’t think it sounds like that discussion is a prerequisite to merging this, so I’ll stamp it approved.

@rapids-bot rapids-bot bot merged commit cb77046 into rapidsai:branch-25.02 Jan 9, 2025
125 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke left review comments

@vyasr vyasr vyasr left review comments

@wence- wence- wence- left review comments

@galipremsagar galipremsagar galipremsagar approved these changes

@bdice bdice bdice approved these changes

@msarahan msarahan Awaiting requested review from msarahan msarahan is a code owner automatically assigned from rapidsai/packaging-codeowners

@shrshi shrshi Awaiting requested review from shrshi shrshi was automatically assigned from rapidsai/cudf-cpp-codeowners

@kingcrimsontianyu kingcrimsontianyu Awaiting requested review from kingcrimsontianyu kingcrimsontianyu was automatically assigned from rapidsai/cudf-cpp-codeowners

Assignees

@Matt711 Matt711

Labels
cudf.polars Issues specific to cudf.polars improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API.
Projects
cuDF Python
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Matt711 @vyasr @wence- @bdice @mroeschke @galipremsagar