closes #79 (#80)

* closes #79

* test grouped_df where idx is specified directly in runner call

* add tests to grouped_df

* update news about defunct arg

* reload docs after removing type argument

* convince codecov that grouped_df is covered

* release

DESCRIPTION

@@ -1,7 +1,7 @@
 Package: runner
 Title: Running Operations for Vectors
 Type: Package
-Version: 0.4.0
+Version: 0.4.1
 Depends: R (>= 3.0)
 Language: en-US
 Encoding: UTF-8

NEWS.md

@@ -1,3 +1,7 @@
+# runner 0.4.1
+* Fix `runner.grouped_df` (`dplyr` class) to not ignore `k` argument.
+* removed defunct argument `type`.
+
 # runner 0.4.0
 * defunct `type` argument in favor of `simplify`.
 * fixed error when using `runner::runner`.

R/run.R

@@ -1,7 +1,7 @@
 #' Apply running function
 #'
 #' Applies custom function on running windows.
-#' @param x (`vector`, `data.frame`, `matrix`, `xts`)\cr
+#' @param x (`vector`, `data.frame`, `matrix`, `xts`, `grouped_df`)\cr
 #'  Input in runner custom function `f`.
 #'
 #' @param k (`integer` vector or single value)\cr
@@ -42,9 +42,6 @@
 #'  Whether incomplete window should return `NA` (if `na_pad = TRUE`)
 #'  Incomplete window is when some parts of the window are out of range.
 #'
-#' @param type (defunct)\cr
-#'  argument defunct from version 0.4.0. Use `simplify` instead.
-#'
 #' @param simplify (`logical` or `character` value)\cr
 #'  should the result be simplified to a vector, matrix or higher dimensional
 #'  array if possible. The default value, `simplify = TRUE`, returns a vector or
@@ -57,7 +54,6 @@
 #'  Create and pass the cluster to the `runner` function to run each window
 #'  calculation in parallel. See \code{\link[parallel]{makeCluster}} in details.
 #'
-#'
 #' @param ... (optional)\cr
 #'   other arguments passed to the function `f`.
 #'
@@ -149,18 +145,10 @@ runner <- function(
   idx = integer(0),
   at = integer(0),
   na_pad = FALSE,
-  type,
   simplify = TRUE,
   cl = NULL,
   ...
   ) {
-  if (!missing(type)) {
-    warning(
-      "Argument 'type' is defunct and will be completely removed in the next release. #nolint
-      Please use 'simplify' argument to manage the output type."
-    )
-  }
-
   UseMethod("runner", x)
 }
 
@@ -235,7 +223,6 @@ runner.default <- function(  #nolint
   idx = integer(0),
   at = integer(0),
   na_pad = FALSE,
-  type,
   simplify = TRUE,
   cl = NULL,
   ...
@@ -339,7 +326,6 @@ runner.data.frame <- function( #nolint
   idx = integer(0),
   at = integer(0),
   na_pad = FALSE,
-  type,
   simplify = TRUE,
   cl = NULL,
   ...
@@ -419,14 +405,14 @@ runner.grouped_df <- function(
   idx = integer(0),
   at = integer(0),
   na_pad = FALSE,
-  type,
   simplify = TRUE,
   cl = NULL,
   ...
 ) {
   runner.data.frame(
     x = this_group(x),
     f = f,
+    k = k,
     lag = lag,
     idx = idx,
     at = at,
@@ -459,7 +445,6 @@ runner.matrix <- function(
   idx = integer(0),
   at = integer(0),
   na_pad = FALSE,
-  type,
   simplify = TRUE,
   cl = NULL,
   ...
@@ -533,7 +518,6 @@ runner.xts <- function(
   idx = integer(0),
   at = integer(0),
   na_pad = FALSE,
-  type,
   simplify = TRUE,
   cl = NULL,
   ...

inst/tinytest/test_external_compatibility.R

@@ -42,8 +42,6 @@ expect_identical(
   "index"
 )
 
-
-# correct grouping results ------
 data <- data.frame(
   index = c(2, 5, 5, 7, 10, 12, 12, 12, 15, 16, 19,
             20, 20, 21, 23, 23, 25, 27, 27, 28),
@@ -52,34 +50,127 @@ data <- data.frame(
   x = 1:20
 )
 
-# running on grouped_df
+# single column in mutate is grouped ------
+res <- data %>%
+  group_by(group1) %>%
+  mutate(
+    m = length(x)
+  )
+
+expect_true(
+  all(res$m == 20)
+)
+
+# running on grouped_df index set ------
 res <- data %>%
   dplyr::group_by(group1, group2) %>%
   run_by(idx = "index") %>%
   dplyr::mutate(
-    x = runner(
+    xx = runner(
       .,
-      f = function(x) {
-        unique(paste(x$group1, x$group2))
+      f = function(df) {
+        paste(df$x, collapse = " ")
       }
     )
   )
+
+expected <- unlist(
+  use.names = FALSE,
+  tapply(
+    data$x,
+    paste(data$group1, data$group2),
+    runner,
+    f = paste,
+    collapse = " "
+  )
+)
+
 expect_identical(
-  res$x,
-  paste(data$group1, data$group2)
+  res$xx,
+  expected
 )
 
-# running on columns from grouped_df
+
+# running on columns from grouped_df k without idx -----
 grouped_dplyr <- data %>%
   dplyr::group_by(group1, group2) %>%
-  run_by(idx = "index") %>%
   dplyr::mutate(
     xx = runner(
-      x = unique(paste(group1, group2)),
-      f = paste, collapse = "")
+      x = .,
+      f = function(df) {
+        paste(df$x, collapse = " ")
+      },
+      k = 2
+    )
+  )
+
+expected <- unlist(
+  use.names = FALSE,
+  tapply(
+    data$x,
+    paste(data$group1, data$group2),
+    runner,
+    f = paste,
+    collapse = " ",
+    k = 2
   )
+)
 
 expect_equal(
   grouped_dplyr$xx,
-  paste(data$group1, data$group2)
+  expected
+)
+
+# running on columns from grouped_df k with idx -----
+grouped_dplyr <- data %>%
+  dplyr::group_by(group1, group2) %>%
+  dplyr::mutate(
+    xx = runner.grouped_df(
+      x = .,
+      f = function(df) {
+        paste(df$x, collapse = " ")
+      },
+      k = 2,
+      idx = index
+    )
+  )
+
+grouped_dplyr2 <- data %>%
+  dplyr::group_by(group1, group2) %>%
+  run_by(idx = "index") %>%
+  dplyr::mutate(
+    xx = runner.grouped_df(
+      x = .,
+      f = function(df) {
+        paste(df$x, collapse = " ")
+      },
+      k = 2
+    )
+  )
+
+expected <- unlist(
+  use.names = FALSE,
+  tapply(
+    1:40,
+    paste(data$group1, data$group2),
+    function(idx) {
+      runner(
+        data$x[idx],
+        f = paste,
+        collapse = " ",
+        k = 2,
+        idx = data$index[idx]
+      )
+    }
+  )
+)
+
+expect_equal(
+  grouped_dplyr$xx,
+  expected
+)
+
+expect_equal(
+  grouped_dplyr2$xx,
+  expected
 )

inst/tinytest/test_runner.R

@@ -1347,12 +1347,3 @@ expect_error(
   runner(1:10, lag = rep(5, 10), idx = 1:10, at = c(4, 5), f = mean),
   "length\\(lag\\) should be 1 or equal to"
 )
-
-#Test defunct -------
-expect_warning(
-  runner(
-    1:10,
-    f = function(x) x,
-    type = "numeric"
-  )
-)