Skip to content

Commit 71caefc

Browse files
committed
fix: address PR review feedback (doctest fixes, edge case handling, formatting)
1 parent aa8694c commit 71caefc

File tree

3 files changed

+19
-10
lines changed

3 files changed

+19
-10
lines changed

src/daft-functions-temporal/src/date_construction.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,12 @@ impl ScalarUDF for MakeTimestamp {
151151
.map(|(((((y, mo), d), h), mi), s)| match (y, mo, d, h, mi, s) {
152152
(Some(y), Some(mo), Some(d), Some(h), Some(mi), Some(s)) => {
153153
let whole_secs = s as u32;
154-
let frac_micros = ((s - whole_secs as f64) * 1_000_000.0).round() as u32;
154+
let frac_micros_raw = ((s - whole_secs as f64) * 1_000_000.0).round() as u32;
155+
let (whole_secs, frac_micros) = if frac_micros_raw >= 1_000_000 {
156+
(whole_secs + 1, 0)
157+
} else {
158+
(whole_secs, frac_micros_raw)
159+
};
155160
let date = NaiveDate::from_ymd_opt(y, mo as u32, d as u32)?;
156161
let time = chrono::NaiveTime::from_hms_micro_opt(
157162
h as u32,
@@ -306,7 +311,12 @@ impl ScalarUDF for MakeTimestampLtz {
306311
.map(|(((((y, mo), d), h), mi), s)| match (y, mo, d, h, mi, s) {
307312
(Some(y), Some(mo), Some(d), Some(h), Some(mi), Some(s)) => {
308313
let whole_secs = s as u32;
309-
let frac_micros = ((s - whole_secs as f64) * 1_000_000.0).round() as u32;
314+
let frac_micros_raw = ((s - whole_secs as f64) * 1_000_000.0).round() as u32;
315+
let (whole_secs, frac_micros) = if frac_micros_raw >= 1_000_000 {
316+
(whole_secs + 1, 0)
317+
} else {
318+
(whole_secs, frac_micros_raw)
319+
};
310320
let date = NaiveDate::from_ymd_opt(y, mo as u32, d as u32)?;
311321
let time = chrono::NaiveTime::from_hms_micro_opt(
312322
h as u32,

src/daft-functions-temporal/src/date_navigation.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,17 +42,16 @@ impl ScalarUDF for LastDay {
4242
let values: Vec<Option<i32>> = arr
4343
.iter()
4444
.map(|opt_days| {
45-
opt_days.map(|days| {
45+
opt_days.and_then(|days| {
4646
let d = days_to_date(days);
4747
let (y, m) = (d.year(), d.month());
48-
let last = if m == 12 {
48+
let first_of_next = if m == 12 {
4949
NaiveDate::from_ymd_opt(y + 1, 1, 1)
5050
} else {
5151
NaiveDate::from_ymd_opt(y, m + 1, 1)
52-
}
53-
.unwrap()
54-
- chrono::Duration::days(1);
55-
date_to_days(last)
52+
}?;
53+
let last = first_of_next - chrono::Duration::days(1);
54+
Some(date_to_days(last))
5655
})
5756
})
5857
.collect();

src/daft-functions-temporal/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ mod unix_timestamp;
1111

1212
use common_error::{DaftResult, ensure};
1313
use current::{CurrentDate, CurrentTimestamp, CurrentTimezone};
14-
use date_construction::{MakeDate, MakeTimestamp, MakeTimestampLtz};
15-
use date_navigation::{LastDay, NextDay};
1614
use daft_core::{
1715
prelude::{DataType, Field, Schema},
1816
series::Series,
@@ -22,6 +20,8 @@ use daft_dsl::{
2220
functions::{FunctionArgs, FunctionModule, FunctionRegistry, ScalarUDF, UnaryArg},
2321
};
2422
use date_arithmetic::{DateAdd, DateDiff, DateSub};
23+
use date_construction::{MakeDate, MakeTimestamp, MakeTimestampLtz};
24+
use date_navigation::{LastDay, NextDay};
2525
use epoch_conversions::{
2626
DateFromUnixDate, FromUnixtime, TimestampMicros, TimestampMillis, TimestampSeconds,
2727
};

0 commit comments

Comments
 (0)