-
Couldn't load subscription status.
- Fork 13
feat: Introduce lazy validation #179
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 50 53 +3
Lines 2915 2996 +81
=========================================
+ Hits 2915 2996 +81 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @borchero ! I only really looked at the python code so far :) One request: Could take this opportunity to add a validation.md under docs/sites/features ? Particularly with eager / lazy differences, I think users will appreciate docs for this now. Also: We need to review the existing docs, bc this surely changes stuff, e.g. in quickstart
This is still missing, I think this can be reviewed regardless 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @borchero ! LGTM modulo docs changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work @borchero 👏 nothing blocking from my side, looks great.
|
For some reason I'm getting this runtime error when executing the tests locally (osx-arm64):
|
Since CI doesn't fail it does not seem to be an issue with the implementation. Let's debug this offline. |
Motivation
Fixes #93.
Changes
eagerparameter to{Schema,Collection}.{validate,filter}to allow for lazy validationTrueas validation should, by default, serve as a barrierNOTE: The eager validation/filter performance is equivalent (and tends to be slightly faster than before). There are, however, significant performance penalties for lazy validation/filtering in collections as well as a notable performance drop for lazy filtering of schemas. This is likely largely due to pola-rs/polars#24129.