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

fix(rust): Revert "Add RLE to RLE_DICTIONARY encoder" #16113

Merged
merged 1 commit into from
May 8, 2024

Conversation

nameexhaustion
Copy link
Collaborator

Reverts #15959

@ritchie46
Copy link
Member

Yes, I did a git bisect and this was the one. 👍

@nameexhaustion nameexhaustion marked this pull request as ready for review May 8, 2024 08:25
@nameexhaustion
Copy link
Collaborator Author

The test case I'll probably put in a future PR, but for now I've checked locally that this revert fixes it.

@ritchie46 ritchie46 merged commit 5996d1e into main May 8, 2024
25 checks passed
@ritchie46 ritchie46 deleted the revert-15959-rle branch May 8, 2024 08:29
@thalassemia
Copy link
Contributor

So sorry for this and thank you for fixing it! I should have written a test with more random, realistic data so this could have been caught before merging. Could I try this again with more robust testing?

@ritchie46
Copy link
Member

Of course! Don't sweat it. That can happen. 👍

@Alex23rodriguez
Copy link

Alex23rodriguez commented May 10, 2024

Happened to me too. A 1 was getting changed to a 5 somewhere in a 1.5M row dataframe...
It would be good is this was added to the release notes of 0.20.25 so people know not to use 0.20.24

@stinodego stinodego changed the title Revert "feat(rust): Add RLE to RLE_DICTIONARY encoder" fix(rust): Revert "Add RLE to RLE_DICTIONARY encoder" May 10, 2024
@github-actions github-actions bot added fix Bug fix rust Related to Rust Polars labels May 10, 2024
@stinodego
Copy link
Contributor

stinodego commented May 10, 2024

Happend to me to. A 1 was getting changed to a 5 somewhere in a 1.5M row dataframe... It would be good is this was added to the release notes of 0.20.25 so people know not to use 0.20.24

Good point - I updated the release notes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants