fix posixct conv back and forth (#589)

Co-authored-by: eitsupi <ts1s1andn@gmail.com>
pola-rs · Dec 10, 2023 · 17feb90 · 17feb90
1 parent c085447
commit 17feb90
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 31 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -28,6 +28,8 @@
   alternatives to `$str$strptime()` (#558).
 - The `dim()` function for DataFrame and LazyFrame correctly returns integer instead of
   double (#577).
+- The conversion of R's `POSIXct` class to Polars datetime now works correctly with millisecond
+  precision (#589).
 
 # polars 0.11.0
 

diff --git a/src/rust/src/conversion_r_to_s.rs b/src/rust/src/conversion_r_to_s.rs
@@ -212,7 +212,7 @@ fn recursive_robjname2series_tree(x: &Robj, name: &str) -> pl::PolarsResult<Seri
             //todo this could probably in fewer allocations
             let dt = pl::DataType::Datetime(pl::TimeUnit::Milliseconds, tz);
             Ok(SeriesTree::Series(
-                (s.cast(&pl::DataType::Int64)? * 1_000i64).cast(&dt)?,
+                ((s * 1000f64).cast(&pl::DataType::Int64)?).cast(&dt)?,
             ))
         }
         Ok(SeriesTree::Series(s)) if x.inherits("Date") => {

diff --git a/src/rust/src/conversion_s_to_r.rs b/src/rust/src/conversion_s_to_r.rs
@@ -186,19 +186,19 @@ pub fn pl_series_to_list(
                 }),
 
             Datetime(tu, opt_tz) => {
-                let tu_i64: i64 = match tu {
-                    pl::TimeUnit::Nanoseconds => 1_000_000_000,
-                    pl::TimeUnit::Microseconds => 1_000_000,
-                    pl::TimeUnit::Milliseconds => 1_000,
+                let tu_f64: f64 = match tu {
+                    pl::TimeUnit::Nanoseconds => 1_000_000_000.0,
+                    pl::TimeUnit::Microseconds => 1_000_000.0,
+                    pl::TimeUnit::Milliseconds => 1_000.0,
                 };
 
                 //resolve timezone
                 let tz = opt_tz.as_ref().map(|s| s.as_str()).unwrap_or("");
-                s.cast(&Int64)?
-                    .i64()
+                s.cast(&Float64)?
+                    .f64()
                     .map(|ca| {
                         ca.into_iter()
-                            .map(|opt| opt.map(|val| (val / tu_i64) as f64))
+                            .map(|opt| opt.map(|val| val / tu_f64))
                             .collect_robj()
                     })
                     // TODO set_class and set_attrib reallocates the vector, find some way to modify without.

diff --git a/tests/testthat/test-datatype.R b/tests/testthat/test-datatype.R
@@ -32,3 +32,37 @@ test_that("plStruct", {
 
   err_state = result(pl$Struct(bin = pl$Binary, pl$Boolean, bob = 42))
 })
+
+
+test_that("POSIXct data conversion", {
+  expect_identical(
+    pl$lit(as.POSIXct("2022-01-01"))$to_r(),
+    as.POSIXct("2022-01-01")
+  )
+
+  expect_identical(
+    pl$lit(as.POSIXct("2022-01-01", tz = "GMT"))$to_r(),
+    as.POSIXct("2022-01-01", tz = "GMT")
+  )
+
+  expect_identical(
+    pl$lit(as.POSIXct("2022-01-01", tz = "HST"))$to_r(),
+    as.POSIXct("2022-01-01", tz = "HST")
+  )
+
+  expect_identical(
+    pl$lit(as.POSIXct("2022-01-01", tz = "GMT"))$to_r(),
+    as.POSIXct("2022-01-01", tz = "GMT")
+  )
+
+
+  x = as.POSIXct(
+    c(
+      "2020-01-01 13:45:48.343",
+      "2020-01-01 13:45:48.343999"
+    ),
+    tz = "UTC"
+  )
+  # POSIXct is converted to datetime[ms], so sub-ms precision is lost
+  expect_identical(pl$lit(x)$to_r(), as.POSIXct(c("2020-01-01 13:45:48.343", "2020-01-01 13:45:48.343"), tz = "UTC"))
+})
diff --git a/tests/testthat/test-expr_datetime.R b/tests/testthat/test-expr_datetime.R
@@ -1,26 +1,3 @@
-test_that("pl$lit posix", {
-  expect_identical(
-    pl$lit(as.POSIXct("2022-01-01"))$to_r(),
-    as.POSIXct("2022-01-01")
-  )
-
-  expect_identical(
-    pl$lit(as.POSIXct("2022-01-01", tz = "GMT"))$to_r(),
-    as.POSIXct("2022-01-01", tz = "GMT")
-  )
-
-  expect_identical(
-    pl$lit(as.POSIXct("2022-01-01", tz = "HST"))$to_r(),
-    as.POSIXct("2022-01-01", tz = "HST")
-  )
-
-  expect_identical(
-    pl$lit(as.POSIXct("2022-01-01", tz = "GMT"))$to_r(),
-    as.POSIXct("2022-01-01", tz = "GMT")
-  )
-})
-
-
 test_that("pl$date_range", {
   t1 = as.POSIXct("2022-01-01")
   t2 = as.POSIXct("2022-01-02")