-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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: Add dt.replace
#19708
base: main
Are you sure you want to change the base?
feat: Add dt.replace
#19708
Conversation
let ca: Int64Chunked = year | ||
.into_iter() | ||
.zip(month) | ||
.zip(day) | ||
.zip(hour) | ||
.zip(minute) | ||
.zip(second) | ||
.zip(microsecond) | ||
.map(|((((((y, m), d), h), mnt), s), us)| { | ||
if let (Some(y), Some(m), Some(d), Some(h), Some(mnt), Some(s), Some(us)) = | ||
(y, m, d, h, mnt, s, us) | ||
{ | ||
NaiveDate::from_ymd_opt(y, m, d) | ||
.and_then(|nd| nd.and_hms_micro_opt(h, mnt, s, us)) | ||
.map(|ndt| match time_unit { | ||
TimeUnit::Milliseconds => ndt.and_utc().timestamp_millis(), | ||
TimeUnit::Microseconds => ndt.and_utc().timestamp_micros(), | ||
TimeUnit::Nanoseconds => ndt.and_utc().timestamp_nanos_opt().unwrap(), | ||
}) | ||
} else { | ||
None | ||
} | ||
}) | ||
.collect_trusted(); | ||
|
||
let ca = match time_zone { | ||
#[cfg(feature = "timezones")] | ||
Some(_) => { | ||
let mut ca = ca.into_datetime(*time_unit, None); | ||
ca = replace_time_zone(&ca, time_zone, _ambiguous, NonExistent::Raise)?; | ||
ca | ||
}, | ||
_ => { | ||
polars_ensure!( | ||
time_zone.is_none(), | ||
ComputeError: "cannot make use of the `time_zone` argument without the 'timezones' feature enabled." | ||
); | ||
ca.into_datetime(*time_unit, 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.
I moved this code into datetime_series_from_parts in polars_time/src/series/mod.rs for re-use.
@MarcoGorelli I could also perhaps use some help creating tests to cover the |
sure - would it work to start with
and then do |
Thanks--looks like I need to use |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19708 +/- ##
==========================================
- Coverage 79.55% 79.55% -0.01%
==========================================
Files 1544 1545 +1
Lines 213240 213514 +274
Branches 2441 2442 +1
==========================================
+ Hits 169643 169851 +208
- Misses 43048 43113 +65
- Partials 549 550 +1 ☔ View full report in Codecov by Sentry. |
Closes #8879.
Probably can use some improvements on the Rust side (seems to be some code duplication but I'm not sure how to best fix it), so any suggestions are welcome please! @MarcoGorelli I moved around a bit of code that overlapped with some of yours but I think the movements all make sense.
Examples