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

feat!: Bump rust-polars to 0.37.0 #776

Merged
merged 34 commits into from
Feb 6, 2024
Merged

feat!: Bump rust-polars to 0.37.0 #776

merged 34 commits into from
Feb 6, 2024

Conversation

etiennebacher
Copy link
Collaborator

https://github.com/pola-rs/polars/releases/tag/rs-0.37.0

@eitsupi if you already started working on this, I can close this and let you open a new PR

@eitsupi
Copy link
Collaborator

eitsupi commented Feb 5, 2024

Thank you for working on this. I haven't done anything yet and don't have enough time right away, so it would be helpful if you could move forward.

@eitsupi
Copy link
Collaborator

eitsupi commented Feb 5, 2024

I think the toolchain version should be updated.

@eitsupi
Copy link
Collaborator

eitsupi commented Feb 5, 2024

Note: pola-rs/polars#14026 should affect S3 methods for operators.

@eitsupi eitsupi added this to the 0.14 milestone Feb 5, 2024
@etiennebacher
Copy link
Collaborator Author

etiennebacher commented Feb 5, 2024

The only failing tests are related to conversion from arrow to polars. This is probably related to the new binview format. There is a new argument pl_flavor in the rust function to_arrow(). When set to true, it gives the failing tests, but when set to false the conversion generates a segfault. I'll need you for this @eitsupi, I don't know how those internals work with arrow

@eitsupi
Copy link
Collaborator

eitsupi commented Feb 6, 2024

There is a new argument pl_flavor in the rust function to_arrow(). When set to true, it gives the failing tests, but when set to false the conversion generates a segfault.

I think it's probably because the schema and actual type were different that caused the segmentation fault.
I think the schema should be true and false should be selected when actually doing the cast.

@eitsupi
Copy link
Collaborator

eitsupi commented Feb 6, 2024

@etiennebacher I believe I have made corrections so that it passes all tests.
In the meantime, I think we can update the NEWS and merge them, may I ask you to update the NEWS?

@eitsupi eitsupi changed the title Bump rust-polars to 0.37.0 feat!: Bump rust-polars to 0.37.0 Feb 6, 2024
@etiennebacher
Copy link
Collaborator Author

Cool, thanks. Yes I'll take care of the NEWS and I'll add some deprecations warnings. I'll ping you when ready

Copy link
Collaborator

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

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

Thanks!

I'm off to bed, so feel free to merge this in anytime. (we can always open a follow up PRs, so the sooner you merge the better, I think)

@etiennebacher etiennebacher marked this pull request as ready for review February 6, 2024 17:47
@etiennebacher etiennebacher merged commit 8b29b1e into main Feb 6, 2024
33 checks passed
@etiennebacher etiennebacher deleted the rust-polars-0.37.0 branch February 6, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants