From adf49694ff83c7059a842a1f621d86b35ad19243 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Wed, 15 Mar 2017 20:07:20 +0100 Subject: [PATCH 1/8] Update to latest rlang API --- DESCRIPTION | 4 ++-- R/bind.r | 2 +- R/colwise.R | 10 +++++----- R/compat-lazyeval.R | 18 +++++++++--------- R/count-tally.R | 10 +++++----- R/dataframe.R | 8 ++++---- R/distinct.R | 2 +- R/funs.R | 6 +++--- R/group-by.r | 4 ++-- R/group-indices.R | 2 +- R/grouped-df.r | 4 ++-- R/hybrid.R | 6 +++--- R/manip.r | 2 +- R/partial-eval.r | 10 +++++----- R/recode.R | 6 +++--- R/rowwise.r | 2 +- R/select-vars.R | 8 ++++---- R/tbl-cube.r | 8 ++++---- R/tbl-df.r | 12 ++++++------ R/tbl-lazy.R | 16 ++++++++-------- R/tbl-sql.r | 2 +- R/translate-sql-window.r | 4 ++-- R/translate-sql.r | 4 ++-- R/utils-expr.R | 2 +- tests/testthat/helper-hybrid.R | 8 ++++---- 25 files changed, 80 insertions(+), 80 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 3ea8c1ca89..8beeec0b8d 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -21,7 +21,7 @@ Imports: magrittr, methods, utils, - rlang, + rlang (>= 0.0.0.9001), R6, Rcpp (>= 0.12.6), tibble (>= 1.2) @@ -141,5 +141,5 @@ Collate: RoxygenNote: 6.0.1 Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "collate")) Remotes: - hadley/rlang, + hadley/rlang#59, tidyverse/glue diff --git a/R/bind.r b/R/bind.r index 79f52f03f2..6fb5fa75c7 100644 --- a/R/bind.r +++ b/R/bind.r @@ -141,7 +141,7 @@ is_data_list <- function(x) { return(TRUE) # With names - if (any(!have_names(x))) + if (any(!have_name(x))) return(FALSE) # Where each element is an 1d vector or list diff --git a/R/colwise.R b/R/colwise.R index 2be231d733..3fdb025a85 100644 --- a/R/colwise.R +++ b/R/colwise.R @@ -162,7 +162,7 @@ summarize_if <- summarise_if #' @seealso [summarise_all()] #' @export vars <- function(...) { - structure(tidy_quotes(...), class = "col_list") + structure(dots_quosures(...), class = "col_list") } is_col_list <- function(cols) inherits(cols, "col_list") @@ -186,8 +186,8 @@ select_colwise_names <- function(tbl, cols) { apply_vars <- function(funs, vars, tbl) { stopifnot(is_fun_list(funs)) - named_calls <- attr(funs, "have_names") - named_vars <- any(have_names(vars)) + named_calls <- attr(funs, "have_name") + named_vars <- any(have_name(vars)) vars <- select_vars(tbl_vars(tbl), !!! vars, exclude = group_vars(tbl)) out <- vector("list", length(vars) * length(funs)) @@ -233,7 +233,7 @@ apply_vars <- function(funs, vars, tbl) { #' be either a list of expressions or a character vector. #' @export summarise_each <- function(tbl, funs, ...) { - summarise_each_(tbl, funs, tidy_quotes(...)) + summarise_each_(tbl, funs, dots_quosures(...)) } #' @export @@ -269,7 +269,7 @@ mutate_each <- function(tbl, funs, ...) { funs <- funs_(funs) } - mutate_each_(tbl, funs, tidy_quotes(...)) + mutate_each_(tbl, funs, dots_quosures(...)) } #' @export diff --git a/R/compat-lazyeval.R b/R/compat-lazyeval.R index f9bfb7f57f..5c9a25536f 100644 --- a/R/compat-lazyeval.R +++ b/R/compat-lazyeval.R @@ -9,7 +9,7 @@ warn_underscored <- function() { warn(paste( "The underscored versions are deprecated in favour of", "tidy evaluation idioms. Please see the documentation", - "for `tidy_quote()` in rlang" + "for `quosure()` in rlang" )) } warn_text_se <- function() { @@ -20,17 +20,17 @@ warn_text_se <- function() { compat_lazy <- function(lazy, env = caller_env(), warn = TRUE) { if (warn) warn_underscored() - coerce_type(lazy, "tidy_quote", - quote = lazy, + coerce_type(lazy, "quosure", + quosure = lazy, symbol = , - language = quosure(lazy, env), + language = new_quosure(lazy, env), string = { if (warn) warn_text_se() - parse_f(lazy, env) + parse_quosure(lazy, env) }, list = - coerce_class(lazy, "tidy_quote", - lazy = quosure(lazy$expr, lazy$env) + coerce_class(lazy, "quosure", + lazy = new_quosure(lazy$expr, lazy$env) ) ) } @@ -48,7 +48,7 @@ compat_lazy_dots <- function(dots, env, ..., .named = FALSE) { warn <- FALSE } - named <- have_names(dots) + named <- have_name(dots) if (.named && any(!named)) { nms <- map_chr(dots[!named], f_text) names(dots)[!named] <- nms @@ -65,7 +65,7 @@ compat_as_lazy <- function(quo) { )) } compat_as_lazy_dots <- function(...) { - structure(class = "lazy_dots", map(tidy_quotes(...), compat_as_lazy)) + structure(class = "lazy_dots", map(dots_quosures(...), compat_as_lazy)) } diff --git a/R/count-tally.R b/R/count-tally.R index e7fda5f037..b5c52a439b 100644 --- a/R/count-tally.R +++ b/R/count-tally.R @@ -64,12 +64,12 @@ tally <- function(x, wt = NULL, sort = FALSE) { inform("Using `n` as weighting variable") wt <- ~n } else { - wt <- tidy_capture(wt) + wt <- arg_quosure(wt) } # Check for NULL lazily, because `wt` could be a tidy-quoted NULL if # add_tally() is called from another function (e.g. add_count()) - n <- tidy_quote( + n <- quosure( if (is_null(!! wt)) { n() } else { @@ -128,12 +128,12 @@ add_tally <- function(x, wt = NULL, sort = FALSE) { inform("Using `n` as weighting variable") wt <- ~n } else { - wt <- tidy_capture(wt) + wt <- arg_quosure(wt) } # Check for NULL lazily, because `wt` could be a tidy-quoted NULL if # add_tally() is called from another function (e.g. add_count()) - n <- tidy_quote( + n <- quosure( if (is_null(!! wt)) { n() } else { @@ -164,7 +164,7 @@ add_count <- function(x, ..., wt = NULL, sort = FALSE) { g <- group_vars(x) grouped <- group_by(x, ..., add = TRUE) - out <- add_tally(grouped, wt = !! tidy_capture(wt), sort = sort) + out <- add_tally(grouped, wt = !! arg_quosure(wt), sort = sort) grouped_df(out, g) } #' @rdname se-deprecated diff --git a/R/dataframe.R b/R/dataframe.R index 4b5d5571ce..02a128e211 100644 --- a/R/dataframe.R +++ b/R/dataframe.R @@ -68,7 +68,7 @@ filter_.data.frame <- function(.data, ..., .dots = list()) { #' @export slice.data.frame <- function(.data, ...) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) slice_impl(.data, dots) } #' @export @@ -192,19 +192,19 @@ distinct_.data.frame <- function(.data, ..., .dots = list(), .keep_all = FALSE) #' @export do.data.frame <- function(.data, ...) { - args <- tidy_quotes(...) + args <- dots_quosures(...) named <- named_args(args) # Create custom dynamic scope with `.` pronoun overscope <- child_env(data = list(. = .data, .data = .data)) if (!named) { - out <- tidy_eval_(args[[1]], overscope) + out <- eval_tidy_(args[[1]], overscope) if (!inherits(out, "data.frame")) { abort("Result must be a data frame") } } else { - out <- map(args, function(arg) list(tidy_eval_(arg, overscope))) + out <- map(args, function(arg) list(eval_tidy_(arg, overscope))) names(out) <- names(args) out <- tibble::as_tibble(out, validate = FALSE) } diff --git a/R/distinct.R b/R/distinct.R index 5d9bd64117..753bf8dad1 100644 --- a/R/distinct.R +++ b/R/distinct.R @@ -44,7 +44,7 @@ distinct_ <- function(.data, ..., .dots, .keep_all = FALSE) { #' vars (character vector) comes out. #' @noRd distinct_vars <- function(.data, ..., .dots, .keep_all = FALSE) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) # If no input, keep all variables if (length(dots) == 0) { diff --git a/R/funs.R b/R/funs.R index 2756237764..d915cfa913 100644 --- a/R/funs.R +++ b/R/funs.R @@ -22,7 +22,7 @@ #' fs <- c("min", "max") #' funs_(fs) funs <- function(..., .args = list()) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) dots <- map(dots, funs_make_call, args = .args) names(dots) <- names2(dots) @@ -31,7 +31,7 @@ funs <- function(..., .args = list()) { names(dots)[missing_names] <- default_names class(dots) <- "fun_list" - attr(dots, "have_names") <- any(!missing_names) + attr(dots, "have_name") <- any(!missing_names) dots } @@ -95,7 +95,7 @@ funs_make_call <- function(x, args, env = base_env()) { expr <- get_expr(x) expr <- switch_type(expr, "funs", - quote = , + quosure = , language = expr, symbol = substitute(f(.), list(f = expr)), string = substitute(f(.), list(f = symbol(expr))) diff --git a/R/group-by.r b/R/group-by.r index 4776aa7940..d5a4713efa 100644 --- a/R/group-by.r +++ b/R/group-by.r @@ -95,11 +95,11 @@ group_by_ <- function(.data, ..., .dots = list(), add = FALSE) { #' @export #' @keywords internal group_by_prepare <- function(.data, ..., .dots = list(), add = FALSE) { - new_groups <- c(tidy_quotes(...), .dots) + new_groups <- c(dots_quosures(...), .dots) # If any calls, use mutate to add new columns, then group by those is_symbol <- map_lgl(new_groups, is_symbol) - named <- have_names(new_groups) + named <- have_name(new_groups) needs_mutate <- named | !is_symbol if (any(needs_mutate)) { diff --git a/R/group-indices.R b/R/group-indices.R index 9e40779ec3..2aaa0f6599 100644 --- a/R/group-indices.R +++ b/R/group-indices.R @@ -24,7 +24,7 @@ group_indices_ <- function(.data, ..., .dots = list()) { #' @export group_indices.data.frame <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) if (length(dots) == 0L) { return(rep(1L, nrow(.data))) } diff --git a/R/grouped-df.r b/R/grouped-df.r index ad7f74c675..f0af625325 100644 --- a/R/grouped-df.r +++ b/R/grouped-df.r @@ -171,7 +171,7 @@ do.grouped_df <- function(.data, ...) { # Create ungroup version of data frame suitable for subsetting group_data <- ungroup(.data) - args <- tidy_quotes(...) + args <- dots_quosures(...) named <- named_args(args) env <- child_env(NULL) @@ -186,7 +186,7 @@ do.grouped_df <- function(.data, ...) { out <- label_output_list(labels, out, groups(.data)) } else { env_bind(env, list(. = group_data, .data = group_data)) - out <- tidy_eval_(args[[1]], env)[0, , drop = FALSE] + out <- eval_tidy_(args[[1]], env)[0, , drop = FALSE] out <- label_output_dataframe(labels, list(list(out)), groups(.data)) } return(out) diff --git a/R/hybrid.R b/R/hybrid.R index 2e973d3015..3972985a23 100644 --- a/R/hybrid.R +++ b/R/hybrid.R @@ -7,7 +7,7 @@ verify_not_hybrid <- function(x) { } with_hybrid <- function(expr, ...) { - with_hybrid_(tidy_capture(expr), ...) + with_hybrid_(arg_quosure(expr), ...) } with_hybrid_ <- function(expr, ...) { @@ -24,7 +24,7 @@ with_hybrid_ <- function(expr, ...) { } without_hybrid <- function(expr, ...) { - without_hybrid_(tidy_capture(expr), ...) + without_hybrid_(arg_quosure(expr), ...) } without_hybrid_ <- function(expr, ...) { @@ -41,7 +41,7 @@ without_hybrid_ <- function(expr, ...) { } eval_dots <- function(expr, ...) { - eval_dots_(tidy_capture(expr), ...) + eval_dots_(arg_quosure(expr), ...) } eval_dots_ <- function(expr, ...) { diff --git a/R/manip.r b/R/manip.r index 83e0fb63d8..5342a953c4 100644 --- a/R/manip.r +++ b/R/manip.r @@ -239,7 +239,7 @@ transmute_ <- function(.data, ..., .dots = list()) { #' @export transmute.default <- function(.data, ...) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) out <- mutate(.data, !!! dots) keep <- names(dots) diff --git a/R/partial-eval.r b/R/partial-eval.r index c0267afda5..2891f218dc 100644 --- a/R/partial-eval.r +++ b/R/partial-eval.r @@ -57,7 +57,7 @@ partial_eval <- function(call, vars = character(), env = caller_env()) { complex = , string = , character = call, - quote = set_expr(call, partial_eval(f_rhs(call), vars, f_env(call))), + quosure = set_expr(call, partial_eval(f_rhs(call), vars, f_env(call))), list = { if (inherits(call, "lazy_dots")) { call <- compat_lazy_dots(call, env) @@ -73,7 +73,7 @@ sym_partial_eval <- function(call, vars, env) { if (name %in% vars) { call } else if (env_has(env, name, inherit = TRUE)) { - expr_eval(call, env) + eval_bare(call, env) } else { call } @@ -84,16 +84,16 @@ lang_partial_eval <- function(call, vars, env) { # Evaluate locally if complex CAR inlined = , namespaced = , - recursive = expr_eval(call, env), + recursive = eval_bare(call, env), named = { # Process call arguments recursively, unless user has manually called # remote/local name <- as_name(node_car(call)) if (name == "local") { - expr_eval(call[[2]], env) + eval_bare(call[[2]], env) } else if (name %in% c("$", "[[", "[")) { # Subsetting is always done locally - expr_eval(call, env) + eval_bare(call, env) } else if (name == "remote") { call[[2]] } else { diff --git a/R/recode.R b/R/recode.R index dbb2a00e8f..59843b91bb 100644 --- a/R/recode.R +++ b/R/recode.R @@ -83,7 +83,7 @@ recode <- function(.x, ..., .default = NULL, .missing = NULL, .dots = NULL) { recode.numeric <- function(.x, ..., .default = NULL, .missing = NULL, .dots = NULL) { values <- c(list(...), .dots) - nms <- have_names(values) + nms <- have_name(values) if (all(nms)) { vals <- as.double(names(values)) } else if (all(!nms)) { @@ -114,7 +114,7 @@ recode.numeric <- function(.x, ..., .default = NULL, .missing = NULL, .dots = NU #' @export recode.character <- function(.x, ..., .default = NULL, .missing = NULL, .dots = NULL) { values <- c(list(...), .dots) - if (!all(have_names(values))) { + if (!all(have_name(values))) { stop("All replacements must be named", call. = FALSE) } @@ -141,7 +141,7 @@ recode.factor <- function(.x, ..., .default = NULL, .missing = NULL, .dots = NUL stop("No replacements provided", call. = FALSE) } - if (!all(have_names(values))) { + if (!all(have_name(values))) { stop("All replacements must be named", call. = FALSE) } if (!is.null(.missing)) { diff --git a/R/rowwise.r b/R/rowwise.r index 7b962d8233..5726351398 100644 --- a/R/rowwise.r +++ b/R/rowwise.r @@ -76,7 +76,7 @@ do.rowwise_df <- function(.data, ...) { group_data <- ungroup(.data) index <- attr(.data, "indices") - args <- tidy_quotes(...) + args <- dots_quosures(...) named <- named_args(args) # Create new environment, inheriting from parent, with an active binding diff --git a/R/select-vars.R b/R/select-vars.R index 221ccc0b8b..1fbc322628 100644 --- a/R/select-vars.R +++ b/R/select-vars.R @@ -42,7 +42,7 @@ #' select_vars(names(iris), !!! list(~Petal.Length)) #' select_vars(names(iris), !! quote(Petal.Length)) select_vars <- function(vars, ..., include = character(), exclude = character()) { - args <- tidy_quotes(...) + args <- dots_quosures(...) if (is_empty(args)) { vars <- setdiff(include, exclude) @@ -62,8 +62,8 @@ select_vars <- function(vars, ..., include = character(), exclude = character()) # Evaluate symbols in an environment where columns are bound, but # not calls (select helpers are scoped in the calling environment) is_helper <- map_lgl(args, function(x) is_lang(x) && !is_lang(x, c("-", ":"))) - ind_list <- map_if(args, is_helper, tidy_eval) - ind_list <- map_if(ind_list, !is_helper, tidy_eval, names_list) + ind_list <- map_if(args, is_helper, eval_tidy) + ind_list <- map_if(ind_list, !is_helper, eval_tidy, names_list) ind_list <- c(initial_case, ind_list) names(ind_list) <- c(names2(initial_case), names2(args)) @@ -114,7 +114,7 @@ setdiff2 <- function(x, y) { #' @export #' @rdname select_vars rename_vars <- function(vars, ...) { - args <- tidy_quotes(...) + args <- dots_quosures(...) if (any(names2(args) == "")) { abort("All arguments to `rename()` must be named.") } diff --git a/R/tbl-cube.r b/R/tbl-cube.r index 24c9390a80..49e9a23b28 100644 --- a/R/tbl-cube.r +++ b/R/tbl-cube.r @@ -319,11 +319,11 @@ rename_.tbl_cube <- function(.data, ..., .dots = list()) { #' @export filter.tbl_cube <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) idx <- map_int(dots, function(d) find_index_check(f_rhs(d), names(.data$dims))) for (i in seq_along(dots)) { - sel <- tidy_eval(dots[[i]], .data$dims) + sel <- eval_tidy(dots[[i]], .data$dims) sel <- sel & !is.na(sel) .data$dims[[idx[i]]] <- .data$dims[[idx[i]]][sel] @@ -389,7 +389,7 @@ group_vars.tbl_cube <- function(x) { #' @export summarise.tbl_cube <- function(.data, ...) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) out_dims <- .data$dims[.data$groups] n <- map_int(out_dims, length) @@ -411,7 +411,7 @@ summarise.tbl_cube <- function(.data, ...) { # Loop over each expression for (j in seq_along(dots)) { - res <- tidy_eval(dots[[j]], mets) + res <- eval_tidy(dots[[j]], mets) out_mets[[j]][i] <- res } } diff --git a/R/tbl-df.r b/R/tbl-df.r index fa433c9789..fb609a83a7 100644 --- a/R/tbl-df.r +++ b/R/tbl-df.r @@ -42,7 +42,7 @@ as.data.frame.tbl_df <- function(x, row.names = NULL, optional = FALSE, ...) { #' @export arrange.tbl_df <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) arrange_impl(.data, dots) } #' @export @@ -53,8 +53,8 @@ arrange_.tbl_df <- function(.data, ..., .dots = list()) { #' @export filter.tbl_df <- function(.data, ...) { - dots <- tidy_quotes(...) - if (any(have_names(dots))) { + dots <- dots_quosures(...) + if (any(have_name(dots))) { abort("filter() takes unnamed arguments. Do you need `==`?") } dots <- exprs_auto_name(dots) @@ -68,7 +68,7 @@ filter_.tbl_df <- function(.data, ..., .dots = list()) { #' @export slice.tbl_df <- function(.data, ...) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) slice_impl(.data, dots) } #' @export @@ -79,7 +79,7 @@ slice_.tbl_df <- function(.data, ..., .dots = list()) { #' @export mutate.tbl_df <- function(.data, ...) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) mutate_impl(.data, dots) } #' @export @@ -90,7 +90,7 @@ mutate_.tbl_df <- function(.data, ..., .dots = list()) { #' @export summarise.tbl_df <- function(.data, ...) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) summarise_impl(.data, dots) } #' @export diff --git a/R/tbl-lazy.R b/R/tbl-lazy.R index 06d08f104f..0335183ec0 100644 --- a/R/tbl-lazy.R +++ b/R/tbl-lazy.R @@ -40,7 +40,7 @@ print.tbl_lazy <- function(x, ...) { #' @export filter.tbl_lazy <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) dots <- partial_eval(dots, vars = op_vars(.data)) add_op_single("filter", .data, dots = dots) } @@ -53,7 +53,7 @@ filter_.tbl_lazy <- function(.data, ..., .dots = list()) { #' @export arrange.tbl_lazy <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) dots <- partial_eval(dots, vars = op_vars(.data)) names(dots) <- NULL @@ -67,7 +67,7 @@ arrange_.tbl_lazy <- function(.data, ..., .dots = list()) { #' @export select.tbl_lazy <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) add_op_single("select", .data, dots = dots) } #' @export @@ -78,7 +78,7 @@ select_.tbl_lazy <- function(.data, ..., .dots = list()) { #' @export rename.tbl_lazy <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) dots <- partial_eval(dots, vars = op_vars(.data)) add_op_single("rename", .data, dots = dots) } @@ -91,7 +91,7 @@ rename_.tbl_lazy <- function(.data, ..., .dots = list()) { #' @export summarise.tbl_lazy <- function(.data, ...) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) add_op_single("summarise", .data, dots = dots) } #' @export @@ -103,7 +103,7 @@ summarise_.tbl_lazy <- function(.data, ..., .dots = list()) { #' @export mutate.tbl_lazy <- function(.data, ..., .dots = list()) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) dots <- partial_eval(dots, vars = op_vars(.data)) add_op_single("mutate", .data, dots = dots) } @@ -116,7 +116,7 @@ mutate_.tbl_lazy <- function(.data, ..., .dots = list()) { #' @export group_by.tbl_lazy <- function(.data, ..., add = FALSE) { - dots <- tidy_quotes(...) + dots <- dots_quosures(...) dots <- partial_eval(dots, vars = op_vars(.data)) if (length(dots) == 0) { @@ -150,7 +150,7 @@ ungroup.tbl_lazy <- function(x, ...) { #' @export distinct.tbl_lazy <- function(.data, ..., .keep_all = FALSE) { - dots <- tidy_quotes(..., .named = TRUE) + dots <- dots_quosures(..., .named = TRUE) dots <- partial_eval(dots, vars = op_vars(.data)) add_op_single("distinct", .data, dots = dots, args = list(.keep_all = .keep_all)) } diff --git a/R/tbl-sql.r b/R/tbl-sql.r index 815fb678b0..ec6c370ebf 100644 --- a/R/tbl-sql.r +++ b/R/tbl-sql.r @@ -461,7 +461,7 @@ do.tbl_sql <- function(.data, ..., .chunk_size = 1e4L) { return(do(.data, ...)) } - args <- tidy_quotes(...) + args <- dots_quosures(...) named <- named_args(args) # Create data frame of labels diff --git a/R/translate-sql-window.r b/R/translate-sql-window.r index 0cf1d31495..d560d92471 100644 --- a/R/translate-sql-window.r +++ b/R/translate-sql-window.r @@ -221,7 +221,7 @@ common_window_funs <- function() { #' translate_window_where(quote(rank() > cumsum(AB))) translate_window_where <- function(expr, window_funs = common_window_funs()) { switch_type(expr, - quote = translate_window_where(f_rhs(expr), window_funs), + quosure = translate_window_where(f_rhs(expr), window_funs), logical = , integer = , double = , @@ -235,7 +235,7 @@ translate_window_where <- function(expr, window_funs = common_window_funs()) { window_where(as_symbol(name), set_names(list(expr), name)) } else { args <- map(expr[-1], translate_window_where, window_funs = window_funs) - expr <- lang(node_car(expr), .args = map(args, "[[", "expr")) + expr <- new_language(node_car(expr), .args = map(args, "[[", "expr")) window_where( expr = expr, diff --git a/R/translate-sql.r b/R/translate-sql.r index fbf07be001..88f663a9fb 100644 --- a/R/translate-sql.r +++ b/R/translate-sql.r @@ -91,7 +91,7 @@ translate_sql <- function(..., } translate_sql_( - tidy_quotes(...), + dots_quosures(...), con = con, vars_group = vars_group, vars_order = vars_order, @@ -113,7 +113,7 @@ translate_sql_ <- function(dots, stopifnot(is.list(dots)) - if (!any(have_names(dots))) { + if (!any(have_name(dots))) { names(dots) <- NULL } diff --git a/R/utils-expr.R b/R/utils-expr.R index be0ab9bae0..f7b0b17f8b 100644 --- a/R/utils-expr.R +++ b/R/utils-expr.R @@ -23,7 +23,7 @@ node_walk_replace <- function(node, old, new) { expr_substitute <- function(expr, old, new) { expr <- duplicate(expr) switch_type(expr, - quote = , + quosure = , language = node_walk_replace(node_cdr(expr), old, new), symbol = if (identical(expr, old)) return(new) ) diff --git a/tests/testthat/helper-hybrid.R b/tests/testthat/helper-hybrid.R index ba3c1ba0aa..d4291bfd68 100644 --- a/tests/testthat/helper-hybrid.R +++ b/tests/testthat/helper-hybrid.R @@ -7,7 +7,7 @@ expect_predicate <- function(actual, expected) { } check_hybrid_result <- function(expr, ..., expected, test_eval = TRUE) { - check_hybrid_result_(rlang::tidy_capture(expr), ..., expected = expected, test_eval = test_eval) + check_hybrid_result_(rlang::arg_quosure(expr), ..., expected = expected, test_eval = test_eval) } check_hybrid_result_ <- function(expr, ..., expected, test_eval) { @@ -18,7 +18,7 @@ check_hybrid_result_ <- function(expr, ..., expected, test_eval) { } check_not_hybrid_result <- function(expr, ..., expected, test_eval = TRUE) { - check_not_hybrid_result_(rlang::tidy_capture(expr), ..., expected = expected, test_eval = test_eval) + check_not_hybrid_result_(rlang::arg_quosure(expr), ..., expected = expected, test_eval = test_eval) } check_not_hybrid_result_ <- function(expr, ..., expected, test_eval) { @@ -29,7 +29,7 @@ check_not_hybrid_result_ <- function(expr, ..., expected, test_eval) { } expect_hybrid_error <- function(expr, ..., error) { - expect_hybrid_error_(rlang::tidy_capture(expr), ..., error = error) + expect_hybrid_error_(rlang::arg_quosure(expr), ..., error = error) } expect_hybrid_error_ <- function(expr, ..., error) { @@ -40,7 +40,7 @@ expect_hybrid_error_ <- function(expr, ..., error) { } expect_not_hybrid_error <- function(expr, ..., error) { - expect_not_hybrid_error_(rlang::tidy_capture(expr), ..., error = error) + expect_not_hybrid_error_(rlang::arg_quosure(expr), ..., error = error) } expect_not_hybrid_error_ <- function(expr, ..., error) { From 66a74c5a848fec38e6db87d62299759308167fc6 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 17 Mar 2017 13:53:41 +0100 Subject: [PATCH 2/8] Rename symbol() to sym() --- R/colwise.R | 8 ++++---- R/count-tally.R | 6 +++--- R/funs.R | 2 +- R/group-by.r | 2 +- R/manip.r | 2 +- R/pull.R | 2 +- R/tbl-sql.r | 8 ++++---- tests/testthat/test-group-by.r | 4 ++-- 8 files changed, 17 insertions(+), 17 deletions(-) diff --git a/R/colwise.R b/R/colwise.R index 3fdb025a85..4f3eed4b88 100644 --- a/R/colwise.R +++ b/R/colwise.R @@ -118,7 +118,7 @@ probe_colwise_names <- function(tbl, p, ...) { vars <- tbl_vars(tbl) vars <- vars[selected] - symbols(vars) + syms(vars) } #' @rdname summarise_all @@ -170,11 +170,11 @@ select_colwise_names <- function(tbl, cols) { vars <- tbl_vars(tbl) if (is_character(cols)) { - selected <- symbols(cols) + selected <- syms(cols) } else if (is_col_list(cols)) { selected <- cols } else if (is.numeric(cols)) { - selected <- symbols(vars[cols]) + selected <- syms(vars[cols]) } else { abort(".cols should be a character/numeric vector or a columns object") } @@ -195,7 +195,7 @@ apply_vars <- function(funs, vars, tbl) { for (i in seq_along(vars)) { for (j in seq_along(funs)) { - var_sym <- symbol(vars[[i]]) + var_sym <- sym(vars[[i]]) out[[i, j]] <- expr_substitute(funs[[j]], quote(.), var_sym) } } diff --git a/R/count-tally.R b/R/count-tally.R index b5c52a439b..977e9d7c8d 100644 --- a/R/count-tally.R +++ b/R/count-tally.R @@ -81,7 +81,7 @@ tally <- function(x, wt = NULL, sort = FALSE) { out <- summarise(x, !! n_name := !! n) if (sort) { - arrange(out, desc(!! symbol(n_name))) + arrange(out, desc(!! sym(n_name))) } else { out } @@ -111,7 +111,7 @@ count <- function(x, ..., wt = NULL, sort = FALSE) { x <- group_by(x, ..., add = TRUE) x <- tally(x, wt = !! wt, sort = sort) - x <- group_by(x, !!! symbols(groups), add = FALSE) + x <- group_by(x, !!! syms(groups), add = FALSE) x } #' @export @@ -145,7 +145,7 @@ add_tally <- function(x, wt = NULL, sort = FALSE) { out <- mutate(x, !! n_name := !! n) if (sort) { - out <- arrange(out, desc(!! symbol(n_name))) + out <- arrange(out, desc(!! sym(n_name))) } grouped_df(out, group_vars(x)) diff --git a/R/funs.R b/R/funs.R index d915cfa913..7ed3b43d80 100644 --- a/R/funs.R +++ b/R/funs.R @@ -98,7 +98,7 @@ funs_make_call <- function(x, args, env = base_env()) { quosure = , language = expr, symbol = substitute(f(.), list(f = expr)), - string = substitute(f(.), list(f = symbol(expr))) + string = substitute(f(.), list(f = sym(expr))) ) expr <- lang_modify(expr, .args = args) diff --git a/R/group-by.r b/R/group-by.r index d5a4713efa..3d689bb3a6 100644 --- a/R/group-by.r +++ b/R/group-by.r @@ -117,7 +117,7 @@ group_by_prepare <- function(.data, ..., .dots = list(), add = FALSE) { list( data = .data, - groups = symbols(group_names), + groups = syms(group_names), group_names = group_names ) } diff --git a/R/manip.r b/R/manip.r index 5342a953c4..6b14ce8e67 100644 --- a/R/manip.r +++ b/R/manip.r @@ -366,7 +366,7 @@ select_if <- function(.data, .predicate, ...) { } vars <- probe_colwise_names(.data, .predicate, ...) vars <- ensure_grouped_vars(vars, .data, notify = FALSE) - select(.data, !!! symbols(vars)) + select(.data, !!! syms(vars)) } #' @rdname select diff --git a/R/pull.R b/R/pull.R index 0c91e8f313..e82238fb6b 100644 --- a/R/pull.R +++ b/R/pull.R @@ -39,7 +39,7 @@ pull.data.frame <- function(.data, var = -1) { pull.tbl_sql <- function(.data, var = -1) { var <- find_var(var, tbl_vars(.data)) - .data <- select(.data, !! symbol(var)) + .data <- select(.data, !! sym(var)) .data <- collect(.data) .data[[1]] } diff --git a/R/tbl-sql.r b/R/tbl-sql.r index ec6c370ebf..d4131dc95e 100644 --- a/R/tbl-sql.r +++ b/R/tbl-sql.r @@ -382,7 +382,7 @@ collapse.tbl_sql <- function(x, vars = NULL, ...) { }) tbl(x$src, sql) %>% - group_by(!!! symbols(op_grps(x))) %>% + group_by(!!! syms(op_grps(x))) %>% add_op_order(op_sort(x)) } @@ -403,7 +403,7 @@ compute.tbl_sql <- function(x, name = random_table_name(), temporary = TRUE, vars <- op_vars(x) assert_that(all(unlist(indexes) %in% vars)) assert_that(all(unlist(unique_indexes) %in% vars)) - x_aliased <- select(x, !!! symbols(vars)) # avoids problems with SQLite quoting (#1754) + x_aliased <- select(x, !!! syms(vars)) # avoids problems with SQLite quoting (#1754) db_save_query(con, sql_render(x_aliased, con), name = name, temporary = temporary) db_create_indexes(con, name, unique_indexes, unique = TRUE) db_create_indexes(con, name, indexes, unique = FALSE) @@ -412,7 +412,7 @@ compute.tbl_sql <- function(x, name = random_table_name(), temporary = TRUE, }) tbl(x$src, name) %>% - group_by(!!! symbols(op_grps(x))) %>% + group_by(!!! syms(op_grps(x))) %>% add_op_order(op_sort(x)) } @@ -505,7 +505,7 @@ do.tbl_sql <- function(.data, ..., .chunk_size = 1e4L) { } # Create an id for each group - grouped <- chunk %>% group_by(!!! symbols(names(chunk)[gvars])) + grouped <- chunk %>% group_by(!!! syms(names(chunk)[gvars])) index <- attr(grouped, "indices") # zero indexed n <- length(index) diff --git a/tests/testthat/test-group-by.r b/tests/testthat/test-group-by.r index 0666f1e095..9cc31bf522 100644 --- a/tests/testthat/test-group-by.r +++ b/tests/testthat/test-group-by.r @@ -65,7 +65,7 @@ test_that("local group_by preserves variable types", { expected <- data_frame(unique(df_var[[var]]), n = 1L) names(expected)[1] <- var - summarised <- df_var %>% group_by(!! symbol(var)) %>% summarise(n = n()) + summarised <- df_var %>% group_by(!! sym(var)) %>% summarise(n = n()) expect_equal(summarised, expected, info = var) } }) @@ -238,7 +238,7 @@ test_that(paste0("group_by handles encodings for native strings (#1507)"), { for (names_converter in c(enc2native, enc2utf8)) { for (dots_converter in c(enc2native, enc2utf8)) { names(df) <- names_converter(c(special, "Eng")) - res <- group_by(df, !!! symbols(dots_converter(special))) + res <- group_by(df, !!! syms(dots_converter(special))) expect_equal(names(res), names(df)) expect_groups(res, special) } From f1258c407a5c2a0fa972f29e3063c9c6eaa3c62b Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 20 Mar 2017 19:19:34 +0100 Subject: [PATCH 3/8] Update tests now that captured formulas are guarded --- tests/testthat/test-hybrid-traverse.R | 4 ++-- tests/testthat/test-hybrid.R | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-hybrid-traverse.R b/tests/testthat/test-hybrid-traverse.R index 77c6494033..dccc5b14af 100644 --- a/tests/testthat/test-hybrid-traverse.R +++ b/tests/testthat/test-hybrid-traverse.R @@ -187,7 +187,7 @@ test_hybrid <- function(grouping) { expect_equal( test_df %>% grouping %>% - mutate(., f = ~ mean(UQ(var))) %>% + mutate(., f = mean(UQ(var))) %>% select(-e), test_df %>% grouping %>% @@ -508,4 +508,4 @@ test_hybrid <- function(grouping) { test_hybrid(identity) test_hybrid(rowwise) -test_hybrid(. %>% group_by(~ id)) +test_hybrid(. %>% group_by(!! ~id)) diff --git a/tests/testthat/test-hybrid.R b/tests/testthat/test-hybrid.R index fa1b5cbf56..f7c217ce38 100644 --- a/tests/testthat/test-hybrid.R +++ b/tests/testthat/test-hybrid.R @@ -4,10 +4,12 @@ test_that("hybrid evaluation environment is cleaned up (#2358)", { # Can't use pipe here, f and g should have top-level parent.env() df <- data_frame(x = 1) df <- mutate(df, f = list(function(){})) - df <- mutate(df, g = list(UQF(~.))) + df <- mutate(df, g = list(~.)) + df <- mutate(df, h = list(UQF(~.))) expect_environments_clean(df$f[[1]]) expect_environments_clean(df$g[[1]]) + expect_environments_clean(df$h[[1]]) }) test_that("n() and n_distinct() work", { From 03cb7a9407f2b05a2f97dee07fe738f78631d1d3 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 20 Mar 2017 20:33:54 +0100 Subject: [PATCH 4/8] Fix filter scoping now that literals are forwarded in empty env --- src/filter.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/filter.cpp b/src/filter.cpp index 908cc51b94..64a47a32b2 100644 --- a/src/filter.cpp +++ b/src/filter.cpp @@ -102,6 +102,9 @@ template DataFrame filter_grouped_single_env(const Data& gdf, const QuosureList& quosures) { typedef GroupedCallProxy Proxy; Environment env = quosures[0].env(); + // FIXME: hacky fix until we switch to tidyeval + if (((SEXP) env) == R_EmptyEnv) + env = R_BaseEnv; const DataFrame& data = gdf.data(); SymbolVector names(data.names()); @@ -226,6 +229,11 @@ DataFrame filter_not_grouped(DataFrame df, const QuosureList& quosures) { } if (quosures.single_env()) { Environment env = quosures[0].env(); + + // FIXME: hacky fix until we switch to tidyeval + if (((SEXP) env) == R_EmptyEnv) + env = R_BaseEnv; + // a, b, c -> a & b & c Shield call(and_calls(quosures, set, env)); From 539757f3233e29e5f9c82ef82a83897afed11a56 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 20 Mar 2017 22:00:29 +0100 Subject: [PATCH 5/8] Update to latest rlang API --- R/count-tally.R | 6 +++--- R/hybrid.R | 6 +++--- R/utils-expr.R | 2 +- tests/testthat/helper-hybrid.R | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/R/count-tally.R b/R/count-tally.R index 977e9d7c8d..36a9ab0cfb 100644 --- a/R/count-tally.R +++ b/R/count-tally.R @@ -64,7 +64,7 @@ tally <- function(x, wt = NULL, sort = FALSE) { inform("Using `n` as weighting variable") wt <- ~n } else { - wt <- arg_quosure(wt) + wt <- catch_quosure(wt) } # Check for NULL lazily, because `wt` could be a tidy-quoted NULL if @@ -128,7 +128,7 @@ add_tally <- function(x, wt = NULL, sort = FALSE) { inform("Using `n` as weighting variable") wt <- ~n } else { - wt <- arg_quosure(wt) + wt <- catch_quosure(wt) } # Check for NULL lazily, because `wt` could be a tidy-quoted NULL if @@ -164,7 +164,7 @@ add_count <- function(x, ..., wt = NULL, sort = FALSE) { g <- group_vars(x) grouped <- group_by(x, ..., add = TRUE) - out <- add_tally(grouped, wt = !! arg_quosure(wt), sort = sort) + out <- add_tally(grouped, wt = !! catch_quosure(wt), sort = sort) grouped_df(out, g) } #' @rdname se-deprecated diff --git a/R/hybrid.R b/R/hybrid.R index 3972985a23..f9681dcfb4 100644 --- a/R/hybrid.R +++ b/R/hybrid.R @@ -7,7 +7,7 @@ verify_not_hybrid <- function(x) { } with_hybrid <- function(expr, ...) { - with_hybrid_(arg_quosure(expr), ...) + with_hybrid_(catch_quosure(expr), ...) } with_hybrid_ <- function(expr, ...) { @@ -24,7 +24,7 @@ with_hybrid_ <- function(expr, ...) { } without_hybrid <- function(expr, ...) { - without_hybrid_(arg_quosure(expr), ...) + without_hybrid_(catch_quosure(expr), ...) } without_hybrid_ <- function(expr, ...) { @@ -41,7 +41,7 @@ without_hybrid_ <- function(expr, ...) { } eval_dots <- function(expr, ...) { - eval_dots_(arg_quosure(expr), ...) + eval_dots_(catch_quosure(expr), ...) } eval_dots_ <- function(expr, ...) { diff --git a/R/utils-expr.R b/R/utils-expr.R index f7b0b17f8b..1815abd080 100644 --- a/R/utils-expr.R +++ b/R/utils-expr.R @@ -15,7 +15,7 @@ node_walk_replace <- function(node, old, new) { while(!is_null(node)) { switch_expr(node_car(node), language = node_walk_replace(node_cdar(node), old, new), - symbol = if (identical(node_car(node), old)) set_node_car(node, new) + symbol = if (identical(node_car(node), old)) mut_node_car(node, new) ) node <- node_cdr(node) } diff --git a/tests/testthat/helper-hybrid.R b/tests/testthat/helper-hybrid.R index d4291bfd68..ad03b8c583 100644 --- a/tests/testthat/helper-hybrid.R +++ b/tests/testthat/helper-hybrid.R @@ -7,7 +7,7 @@ expect_predicate <- function(actual, expected) { } check_hybrid_result <- function(expr, ..., expected, test_eval = TRUE) { - check_hybrid_result_(rlang::arg_quosure(expr), ..., expected = expected, test_eval = test_eval) + check_hybrid_result_(rlang::catch_quosure(expr), ..., expected = expected, test_eval = test_eval) } check_hybrid_result_ <- function(expr, ..., expected, test_eval) { @@ -18,7 +18,7 @@ check_hybrid_result_ <- function(expr, ..., expected, test_eval) { } check_not_hybrid_result <- function(expr, ..., expected, test_eval = TRUE) { - check_not_hybrid_result_(rlang::arg_quosure(expr), ..., expected = expected, test_eval = test_eval) + check_not_hybrid_result_(rlang::catch_quosure(expr), ..., expected = expected, test_eval = test_eval) } check_not_hybrid_result_ <- function(expr, ..., expected, test_eval) { @@ -29,7 +29,7 @@ check_not_hybrid_result_ <- function(expr, ..., expected, test_eval) { } expect_hybrid_error <- function(expr, ..., error) { - expect_hybrid_error_(rlang::arg_quosure(expr), ..., error = error) + expect_hybrid_error_(rlang::catch_quosure(expr), ..., error = error) } expect_hybrid_error_ <- function(expr, ..., error) { @@ -40,7 +40,7 @@ expect_hybrid_error_ <- function(expr, ..., error) { } expect_not_hybrid_error <- function(expr, ..., error) { - expect_not_hybrid_error_(rlang::arg_quosure(expr), ..., error = error) + expect_not_hybrid_error_(rlang::catch_quosure(expr), ..., error = error) } expect_not_hybrid_error_ <- function(expr, ..., error) { From a592c9b7d766492d3e347aacf5cae1f7a9b688ec Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Mon, 20 Mar 2017 23:01:13 +0100 Subject: [PATCH 6/8] Update rlang revdep --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 8beeec0b8d..70107aec38 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -141,5 +141,5 @@ Collate: RoxygenNote: 6.0.1 Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", "collate")) Remotes: - hadley/rlang#59, + hadley/rlang, tidyverse/glue From f1c674a5ef55cd5d2dbee7b8ef88168816656783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Tue, 21 Mar 2017 08:08:40 +0100 Subject: [PATCH 7/8] Ordered factors can be created by summarise() (#2548) * reenable failing tests * don't overwrite class attribute * NEWS --- NEWS.md | 2 ++ inst/include/dplyr/Result/DelayedProcessor.h | 1 - tests/testthat/test-summarise.r | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 12622d7ab0..18e9b32d2c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # dplyr 0.5.0.9000 +* `summarise()` now can create ordered factors (#2200). + * `bind_rows()` and `bind_cols()` give an error for database tables (#2373). * `bind_rows()` works correctly with `NULL` arguments and an `.id` argument diff --git a/inst/include/dplyr/Result/DelayedProcessor.h b/inst/include/dplyr/Result/DelayedProcessor.h index 5cf846810f..2cf46ca8bd 100644 --- a/inst/include/dplyr/Result/DelayedProcessor.h +++ b/inst/include/dplyr/Result/DelayedProcessor.h @@ -192,7 +192,6 @@ namespace dplyr { levels[it->second-1] = it->first; } set_levels(res, levels); - set_class(res, "factor"); return res; } diff --git a/tests/testthat/test-summarise.r b/tests/testthat/test-summarise.r index 6da5d23089..326d295b32 100644 --- a/tests/testthat/test-summarise.r +++ b/tests/testthat/test-summarise.r @@ -875,7 +875,6 @@ test_that("typing and NAs for rowwise summarise (#1839)", { }) test_that("calculating an ordered factor preserves order (#2200)", { - skip("Currently failing") test_df <- tibble( id = c("a", "b"), val = 1:2 @@ -889,7 +888,6 @@ test_that("calculating an ordered factor preserves order (#2200)", { }) test_that("min, max preserves ordered factor data (#2200)", { - skip("Currently failing") test_df <- tibble( id = rep(c("a", "b"), 2), ord = ordered(c("A", "B", "B", "A"), levels = c("A", "B")) From 4be74ef2b32e77d721d65050c3b02ab032ff7903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Tue, 21 Mar 2017 08:09:38 +0100 Subject: [PATCH 8/8] Copy attributes for summary variables (#2547) * simplify partial specialization * add failing test * copy attributes to output of summary template * NEWS --- NEWS.md | 2 ++ inst/include/dplyr/Result/GroupedSubset.h | 26 ++++++----------------- tests/testthat/test-summarise.r | 11 ++++++++++ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/NEWS.md b/NEWS.md index 18e9b32d2c..93c6bc5aee 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # dplyr 0.5.0.9000 +* `summarise()` now correctly evaluates newly created factors (#2217). + * `summarise()` now can create ordered factors (#2200). * `bind_rows()` and `bind_cols()` give an error for database tables (#2373). diff --git a/inst/include/dplyr/Result/GroupedSubset.h b/inst/include/dplyr/Result/GroupedSubset.h index 07258751fa..d3a042c193 100644 --- a/inst/include/dplyr/Result/GroupedSubset.h +++ b/inst/include/dplyr/Result/GroupedSubset.h @@ -86,7 +86,10 @@ namespace dplyr { typedef typename Rcpp::traits::storage_type::type STORAGE; SummarisedSubsetTemplate(SummarisedVariable x) : - object(x), output(1) {} + object(x), output(1) + { + copy_most_attributes(output, object); + } virtual SEXP get(const SlicingIndex& indices) { output[0] = object[indices.group()]; @@ -105,24 +108,9 @@ namespace dplyr { }; template <> - class SummarisedSubsetTemplate : public GroupedSubset { - public: - SummarisedSubsetTemplate(SummarisedVariable x) : object(x) {} - - virtual SEXP get(const SlicingIndex& indices) { - return List::create(object[indices.group()]); - } - - virtual SEXP get_variable() const { - return object; - } - virtual bool is_summary() const { - return true; - } - - private: - List object; - }; + inline SEXP SummarisedSubsetTemplate::get(const SlicingIndex& indices) { + return List::create(object[indices.group()]); + } inline GroupedSubset* summarised_subset(SummarisedVariable x) { switch (TYPEOF(x)) { diff --git a/tests/testthat/test-summarise.r b/tests/testthat/test-summarise.r index 326d295b32..f0ad7ac5c6 100644 --- a/tests/testthat/test-summarise.r +++ b/tests/testthat/test-summarise.r @@ -44,6 +44,17 @@ test_that("summarise can refer to variables that were just created (#138)", { expect_equal(res$cyl2, res_direct$cyl2) }) +test_that("summarise can refer to factor variables that were just created (#2217)", { + df <- data_frame(a = 1:3) %>% + group_by(a) + res <- df %>% + summarise(f = factor(if_else(a <= 1, "a", "b")), g = (f == "a")) + expect_equal( + res, + data_frame(a = 1:3, f = factor(c("a", "b", "b")), g = c(TRUE, FALSE, FALSE)) + ) +}) + test_that("summarise refuses to modify grouping variable (#143)", { df <- data.frame(a = c(1, 2, 1, 2), b = c(1, 1, 2, 2), x = 1:4) ds <- group_by(tbl_df(df), a, b)