-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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: Ensure pl.datetime returns empty column when input columns are empty #20278
fix: Ensure pl.datetime returns empty column when input columns are empty #20278
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #20278 +/- ##
==========================================
+ Coverage 79.59% 79.61% +0.02%
==========================================
Files 1565 1565
Lines 218462 218344 -118
Branches 2475 2478 +3
==========================================
- Hits 173885 173841 -44
+ Misses 44010 43936 -74
Partials 567 567 ☔ View full report in Codecov by Sentry. |
}, | ||
_ => { | ||
polars_ensure!( | ||
time_zone.is_none(), |
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.
This should be an assert. We panic on missing features.
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 for the info! Updated in 8b48562. The existing polars_ensure
on line 200 has also been replaced with assert
.
@@ -78,6 +81,29 @@ pub(super) fn datetime( | |||
use polars_core::export::chrono::NaiveDate; | |||
use polars_core::utils::CustomIterTools; | |||
|
|||
if s.iter().any(|s| s.is_empty()) { | |||
return Ok(Column::new_empty( | |||
PlSmallStr::from_static("datetime"), |
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.
Can we hoist this into a variable at the start of the function? Then we can reuse that here and on line 207.
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.
Sure. Updated in 8b48562.
Thank you @haocheng6 |
Fixes #19696
In the new code, the returned column is named as "datetime" for consistency. The column name issue of
pl.datetime
is tracked by #18808.