better negation of duration string

pola-rs · May 21, 2024 · d27cd07 · d27cd07
1 parent 9a089bf
commit d27cd07
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 5 deletions.
diff --git a/R/dataframe__frame.R b/R/dataframe__frame.R
@@ -2132,7 +2132,7 @@ DataFrame_rolling = function(
     group_by = NULL,
     check_sorted = TRUE) {
   period = parse_as_polars_duration_string(period)
-  offset = parse_as_polars_duration_string(offset) %||% paste0("-", period)
+  offset = parse_as_polars_duration_string(offset) %||% negate_duration_string(period)
   construct_rolling_group_by(self, index_column, period, offset, closed, group_by, check_sorted)
 }
 
@@ -2216,7 +2216,7 @@ DataFrame_group_by_dynamic = function(
     start_by = "window",
     check_sorted = TRUE) {
   every = parse_as_polars_duration_string(every)
-  offset = parse_as_polars_duration_string(offset) %||% paste0("-", every)
+  offset = parse_as_polars_duration_string(offset) %||% negate_duration_string(every)
   period = parse_as_polars_duration_string(period) %||% every
   construct_group_by_dynamic(
     self, index_column, every, period, offset, include_boundaries, closed, label,

diff --git a/R/expr__expr.R b/R/expr__expr.R
@@ -3321,7 +3321,7 @@ Expr_rolling = function(
     closed = "right",
     check_sorted = TRUE) {
   period = parse_as_polars_duration_string(period)
-  offset = parse_as_polars_duration_string(offset) %||% paste0("-", period)
+  offset = parse_as_polars_duration_string(offset) %||% negate_duration_string(period)
   .pr$Expr$rolling(self, index_column, period, offset, closed, check_sorted) |>
     unwrap("in $rolling():")
 }

diff --git a/R/lazyframe__lazy.R b/R/lazyframe__lazy.R
@@ -1915,7 +1915,7 @@ LazyFrame_rolling = function(
     group_by = NULL,
     check_sorted = TRUE) {
   period = parse_as_polars_duration_string(period)
-  offset = parse_as_polars_duration_string(offset) %||% paste0("-", period)
+  offset = parse_as_polars_duration_string(offset) %||% negate_duration_string(period)
   .pr$LazyFrame$rolling(
     self, index_column, period, offset, closed,
     wrap_elist_result(group_by, str_to_lit = FALSE), check_sorted
@@ -2025,7 +2025,7 @@ LazyFrame_group_by_dynamic = function(
     start_by = "window",
     check_sorted = TRUE) {
   every = parse_as_polars_duration_string(every)
-  offset = parse_as_polars_duration_string(offset) %||% paste0("-", every)
+  offset = parse_as_polars_duration_string(offset) %||% negate_duration_string(every)
   period = parse_as_polars_duration_string(period) %||% every
 
   .pr$LazyFrame$group_by_dynamic(

diff --git a/R/parse_as_duration.R b/R/parse_as_duration.R
@@ -93,3 +93,11 @@ difftime_to_duration_string = function(dft) {
   )
   paste0(value, unit)
 }
+
+negate_duration_string = function(x) {
+  if (startsWith(x, "-")) {
+    gsub("^-", "", x)
+  } else {
+    paste0("-", x)
+  }
+}
diff --git a/tests/testthat/test-groupby.R b/tests/testthat/test-groupby.R
@@ -274,6 +274,18 @@ test_that("group_by_dynamic for LazyFrame: error if not explicitly sorted", {
   )
 })
 
+test_that("group_by_dynamic for LazyFrame: error if every is negative", {
+  df = pl$LazyFrame(
+    idx = 0:5,
+    n = 0:5
+  )$with_columns(pl$col("idx")$set_sorted())
+
+  expect_grepl_error(
+    df$group_by_dynamic("idx", every = "-2i")$agg(pl$col("n")$mean())$collect(),
+    "'every' argument must be positive"
+  )
+})
+
 test_that("group_by_dynamic for LazyFrame: arg 'closed' works", {
   df = pl$LazyFrame(
     dt = c(