Skip to content
This repository was archived by the owner on Jul 28, 2025. It is now read-only.

Conversation

inakineitor
Copy link

I fixed a failing test and upgraded the library to the latest Polars and PyO3 version. Please let me know if there is anything I should change before this can be merged. #136

@DeliciousHair
Copy link

I started doing this exact task and then noticed your PR here--mental note to check this sort of thing before beginning the task.

Regardless, I have used polars 0.47.1 and pyo3-0.24.2 instead, and have managed to get everything except example/extend_polars_python_dispatch/extend_polars to build and pass tests without warnings, but it has involved making some changes that differ from yours.

I don't want to open up a competing PR on what is essentially quibbling on a few lines of code, so I am wondering what the etiquette is for this sort of situation?

@baiguoname
Copy link

rust polars version 0.47.1 have many changes, maybe you should bump to this version.

Copy link
Contributor

@Shoeboxam Shoeboxam left a comment

Choose a reason for hiding this comment

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

Pre-emptive review, in case one of the Polars guys catches this out. Thanks for updating!

@DeliciousHair Given 0.47.1 has many changes, your PR may still be helpful.

@DeliciousHair
Copy link

Pre-emptive review, in case one of the Polars guys catches this out. Thanks for updating!

@DeliciousHair Given 0.47.1 has many changes, your PR may still be helpful.

Thanks, I will open a competing PR now.

Reformatting Cargo.toml

Co-authored-by: Michael Shoemate <[email protected]>
@stijnherfst
Copy link
Collaborator

Hi @inakineitor, we have since merged 0.48 #141. Thank you for your contributions. Both your PRs were helpful to reference as I missed a couple small things. I hope this fixed any issues you had 🙂

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants