fix: datetime operations (e.g. .dt.year) were raising when null values were backed by out-of-range integers #15420
Conversation
…s were backed by out-of-range integers
github-actions
bot
added
fix
| @@ -151,7 +151,7 @@ pub fn timestamp_s_to_datetime_opt(seconds: i64) -> Option<NaiveDateTime> { | |||
| /// converts a `i64` representing a `timestamp(ms)` to [`NaiveDateTime`] | |||
| #[inline] | |||
| pub fn timestamp_ms_to_datetime(v: i64) -> NaiveDateTime { | |||
| timestamp_ms_to_datetime_opt(v).expect("invalid or out-of-range datetime") | |||
| timestamp_ms_to_datetime_opt(v).expect(&format!("invalid or out-of-range datetime: {v}")) | |||
There was a problem hiding this comment.
Yikes. This does a heap allocation and string format for very function call. Which I expect are a lot.
There was a problem hiding this comment.
sorry was just debugging 😄 when I saw what looked like np.NaT, I figured what the issue was
There was a problem hiding this comment.
i.e.:
In [3]: np.datetime64('NaT').astype('int64')
Out[3]: -9223372036854775808
MarcoGorelli
changed the title
MarcoGorelli
requested review from
stinodego,
c-peters,
alexander-beedie,
reswqa and
orlp
as code owners
| .iter() | ||
| .map(|v| v.map(|x| op(func(*x)))) | ||
| .collect::<Vec<_>>(), | ||
| )) |
There was a problem hiding this comment.
I believe you collect a Vec of options here and then create a PrimitiveArray. This will be a double allocation. You can create the array directly from the iterator.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15420 +/- ##
=======================================
Coverage 81.13% 81.13%
=======================================
Files 1362 1362
Lines 174802 174814 +12
Branches 2531 2531
=======================================
+ Hits 141825 141836 +11
Misses 32493 32493
- Partials 484 485 +1 ☔ View full report in Codecov by Sentry. |
closes #15313