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: Fix lazy schema for cut/qcut when allow_breaks=True #11287

Merged
merged 6 commits into from
Oct 4, 2023

Conversation

mcrumiller
Copy link
Contributor

Resolves #11255

@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Sep 24, 2023
@mcrumiller mcrumiller changed the title fix(rust,python): Fix return sig for Cut/QCut when allow_breaks=True fix(rust,python): Fix return sig for cut/qcut when allow_breaks=True Sep 24, 2023
Copy link
Member

@stinodego stinodego left a comment

Choose a reason for hiding this comment

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

Good addition - some minor remarks though about the testing.

@mcrumiller
Copy link
Contributor Author

@stinodego I moved test_cut.py over to test_operations like you suggested. I fleshed out all of the tests a bit more (adding series/expr tests), organized a bit more, and included the schema checks.

Copy link
Member

@stinodego stinodego left a comment

Choose a reason for hiding this comment

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

@mcrumiller I refactored the tests as there were a lot of things duplicated. Try to limit to one assert per test - if you want to reuse code or data, use parametrize or fixtures. Most importantly though, the schema wasn't being checked correctly.

After fixing the tests, it turns out the implementation was incorrect. So after I fixed that, everything now works correctly 🎉

@stinodego stinodego changed the title fix(rust,python): Fix return sig for cut/qcut when allow_breaks=True fix(rust,python): Fix lazy schema for cut/qcut when allow_breaks=True Oct 4, 2023
@stinodego stinodego changed the title fix(rust,python): Fix lazy schema for cut/qcut when allow_breaks=True fix: Fix lazy schema for cut/qcut when allow_breaks=True Oct 4, 2023
@stinodego stinodego merged commit c293fb2 into pola-rs:main Oct 4, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lazy frame cannot access cut that includes breaks
3 participants