[Fix #1005] Implement duration -> as.character -> as.duration roundtrip

tidyverse · Jan 12, 2022 · 46d8d19 · 46d8d19
1 parent 5b9eec8
commit 46d8d19
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 17 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -3,6 +3,7 @@ Version 1.8.0.9000
 
 ### NEW FEATURES
 
+* [#1005](https://github.com/tidyverse/lubridate/issues/1005) `as.duration` now allows for full roundtrip `duration -> as.character -> as.duration`
 * [#911](https://github.com/tidyverse/lubridate/issues/911) C parsers treat multiple spaces as one (just like strptime does)
 * `stamp` gained new argument `exact=FALSE` to indicate whether `orders` argument is an exact strptime formats string or not.
 * [#1001](https://github.com/tidyverse/lubridate/issues/1001) Add `%within` method with signature (Interval, list), which was documented but not implemented.

diff --git a/src/period.c b/src/period.c
@@ -45,35 +45,40 @@ static const char *PERIOD_UNITS[] = {"seconds", "minutes", "hours",
                                      "days", "weeks", "months", "years"};
 #define N_PERIOD_UNITS 7
 
-fractionUnit parse_period_unit (const char **c) {
+fractionUnit parse_period_unit(const char **c) {
+  // assumes we are at the beg of a alpha-numeric input
   // units: invalid=-1, S=0,  M=1, H=2, d=3, w=4, m=5, y=6
-  // SKIP_NON_ALPHANUMS(*c);   //  why this macro doesn't work here?
-  while(**c && !(ALPHA(**c) || DIGIT(**c) || **c == '.')) (*c)++;;
+  while(**c && !(ALPHA(**c) || DIGIT(**c) || **c == '.')) (*c)++;
 
   fractionUnit out;
   out.unit = -1;
-  out.val = parse_int(c, 100, FALSE);
-  if (**c == '.') {
-    (*c)++;
-    // allow fractions without leading 0
-    if (out.val == -1)
-      out.val = 0;
-    out.fraction = parse_fractional(c);
-  } else {
-    out.fraction = 0.0;
+  if (**c) {
+    out.val = parse_int(c, 100, FALSE);
+    if (**c == '.') {
+      (*c)++;
+      // allow fractions without leading 0
+      if (out.val == -1)
+        out.val = 0;
+      out.fraction = parse_fractional(c);
+    } else {
+      out.fraction = 0.0;
+    }
   }
 
-  if(**c){
+  if (**c) {
     out.unit = parse_alphanum(c, EN_UNITS, N_EN_UNITS, 0);
     if (out.unit < 0 || out.unit > 16) {
       return out;
     } else {
       // if only unit name supplied, default to 1 units
-      if(out.val == -1)
+      if (out.val == -1)
         out.val = 1;
-      if (out.unit < 3) out.unit = 0;      // seconds
-      else if (out.unit < 6) out.unit = 1; // minutes
-      else if (out.unit < 16) out.unit = (out.unit - 6)/2 + 2;
+      if (out.unit < 3)
+        out.unit = 0; // seconds
+      else if (out.unit < 6)
+        out.unit = 1; // minutes
+      else if (out.unit < 16)
+        out.unit = (out.unit - 6) / 2 + 2;
       return out;
     }
   } else {
@@ -107,7 +112,20 @@ void parse_period_1 (const char **c, double ret[N_PERIOD_UNITS]){
       ret[0] = NA_REAL;
       break;
     }
+
+    while (**c && !(ALPHA(**c) || DIGIT(**c) || **c == '.')) {
+      /* Rprintf("c=%c\n", **c); */
+      if (**c == '(') {
+        // skip till closing ')' to allow for as.duration round-trip #1005
+        while (**c && **c != ')')
+          (*c)++;
+        (*c)++;
+      } else {
+        (*c)++;
+      }
+    }
   }
+
   if (!parsed1) {
     ret[0] = NA_REAL;
   }

diff --git a/tests/testthat/test-durations.R b/tests/testthat/test-durations.R
@@ -131,6 +131,12 @@ test_that("as.duration handles periods", {
   expect_equal(as.duration(seconds(1) + minutes(4)), dseconds(1) + dminutes(4))
 })
 
+test_that("parsing duration's allows for a full roundtrip", {
+  expect_equal(as.duration(lubridate:::parse_period("31557600s (~1 years)")), dseconds(31557600))
+  durs <- duration(c("1m 2s, 3days", "2.5min 1day"))
+  expect_equal(durs, as.duration(as.character(durs)))
+})
+
 test_that("as.duration handles intervals", {
         time1 <- as.POSIXct("2009-01-02 12:24:03", tz = "UTC")
         time2 <- as.POSIXct("2010-02-03 14:31:42", tz = "UTC")