Make usage of across() and data frames in filter() defunct (#7765)

* Advance usage of `across()` and data frames in `filter()` to defunct

By removing support for data frames entirely, so now the incompatible type error is thrown. This nicely simplifies the `filter()` internals.

* Remove unused `column_name` error field

* Provide some extra advice for users of `across()`-in-`filter()`

Author: DavisVaughan

NEWS.md

@@ -125,6 +125,8 @@
 
   * The `keep` argument of `group_map()`, `group_modify()`, and `group_split()`. Deprecated in 1.0.0, use `.keep` instead.
 
+  * Using `across()` and data frames in `filter()`. Deprecated in 1.0.8, use `if_any()` or `if_all()` instead.
+
   * `multiple = NULL` in joins. Deprecated in 1.1.1, use `multiple = "all"` instead.
 
   * `multiple = "error" / "warning"` in joins. Deprecated in 1.1.1, use `relationship = "many-to-one"` instead.

R/filter.R

@@ -231,12 +231,6 @@ filter_eval <- function(
     },
     `dplyr:::signal_filter_one_column_matrix` = function(e) {
       warn_filter_one_column_matrix(env = error_call, user_env = user_env)
-    },
-    `dplyr:::signal_filter_across` = function(e) {
-      warn_filter_across(env = error_call, user_env = user_env)
-    },
-    `dplyr:::signal_filter_data_frame` = function(e) {
-      warn_filter_data_frame(env = error_call, user_env = user_env)
     }
   )
 
@@ -250,18 +244,25 @@ filter_bullets <- function(cnd, ...) {
 
 #' @export
 `filter_bullets.dplyr:::filter_incompatible_type` <- function(cnd, ...) {
-  column_name <- cnd$dplyr_error_data$column_name
   index <- cnd$dplyr_error_data$index
   result <- cnd$dplyr_error_data$result
 
-  if (is.null(column_name)) {
-    input_name <- glue("..{index}")
-  } else {
-    input_name <- glue("..{index}${column_name}")
-  }
-  glue(
-    "`{input_name}` must be a logical vector, not {obj_type_friendly(result)}."
+  bullets <- cli::format_inline(
+    "`..{index}` must be a logical vector, not {obj_type_friendly(result)}."
   )
+
+  if (is.data.frame(result)) {
+    # Provide some extra advice for people who try and use `across()` inside
+    # of `filter()`
+    bullets <- c(
+      bullets,
+      i = cli::format_inline(
+        "If you used {.fn across} to generate this data frame, please use {.fn if_any} or {.fn if_all} instead."
+      )
+    )
+  }
+
+  bullets
 }
 
 #' @export
@@ -283,27 +284,3 @@ warn_filter_one_column_matrix <- function(env, user_env) {
     always = TRUE
   )
 }
-
-warn_filter_across <- function(env, user_env) {
-  # TODO: https://github.com/tidyverse/dplyr/issues/7758
-  lifecycle::deprecate_warn(
-    when = "1.0.8",
-    what = I("Using `across()` in `filter()`"),
-    with = I("`if_any()` or `if_all()`"),
-    always = TRUE,
-    env = env,
-    user_env = user_env
-  )
-}
-
-warn_filter_data_frame <- function(env, user_env) {
-  # TODO: https://github.com/tidyverse/dplyr/issues/7758
-  lifecycle::deprecate_warn(
-    when = "1.0.8",
-    what = I("Returning data frames from `filter()` expressions"),
-    with = I("`if_any()` or `if_all()`"),
-    always = TRUE,
-    env = env,
-    user_env = user_env
-  )
-}

src/dplyr.h

@@ -64,7 +64,6 @@ SEXP eval_tidy(SEXP expr, SEXP data, SEXP env);
 SEXP as_data_pronoun(SEXP x);
 SEXP new_data_mask(SEXP bottom, SEXP top);
 SEXP str_as_symbol(SEXP);
-SEXP quo_get_expr(SEXP quo);
 void env_unbind(SEXP, SEXP);
 }
 

src/filter.cpp

@@ -17,12 +17,10 @@ void stop_filter_incompatible_size(R_xlen_t i,
 static inline
 void stop_filter_incompatible_type(R_xlen_t i,
                                    SEXP quos,
-                                   SEXP column_name,
                                    SEXP result){
-  DPLYR_ERROR_INIT(3);
+  DPLYR_ERROR_INIT(2);
   DPLYR_ERROR_SET(0, "index", Rf_ScalarInteger(i + 1));
-  DPLYR_ERROR_SET(1, "column_name", column_name);
-  DPLYR_ERROR_SET(2, "result", result);
+  DPLYR_ERROR_SET(1, "result", result);
   DPLYR_ERROR_THROW("dplyr:::filter_incompatible_type");
 }
 
@@ -37,14 +35,6 @@ static
 void signal_filter_one_column_matrix() {
   signal_filter("dplyr:::signal_filter_one_column_matrix");
 }
-static
-void signal_filter_across() {
-  signal_filter("dplyr:::signal_filter_across");
-}
-static
-void signal_filter_data_frame() {
-  signal_filter("dplyr:::signal_filter_data_frame");
-}
 
 }
 
@@ -105,41 +95,6 @@ bool filter_is_valid_lgl(SEXP x, bool first) {
   return false;
 }
 
-static inline
-void filter_df_reduce(SEXP x,
-                      R_xlen_t n,
-                      bool first,
-                      R_xlen_t i_quo,
-                      SEXP quos,
-                      int* p_reduced) {
-  if (first) {
-    SEXP expr = rlang::quo_get_expr(VECTOR_ELT(quos, i_quo));
-    const bool across = TYPEOF(expr) == LANGSXP && CAR(expr) == dplyr::symbols::across;
-
-    if (across) {
-      dplyr::signal_filter_across();
-    } else {
-      dplyr::signal_filter_data_frame();
-    }
-  }
-
-  const SEXP* p_x = VECTOR_PTR_RO(x);
-  const R_xlen_t n_col = Rf_xlength(x);
-
-  for (R_xlen_t i = 0; i < n_col; ++i) {
-    SEXP col = p_x[i];
-
-    if (!filter_is_valid_lgl(col, first)) {
-      SEXP names = PROTECT(Rf_getAttrib(x, R_NamesSymbol));
-      SEXP name = PROTECT(Rf_ScalarString(STRING_ELT(names, i)));
-      dplyr::stop_filter_incompatible_type(i_quo, quos, name, col);
-      UNPROTECT(2);
-    }
-
-    filter_lgl_reduce(col, n, p_reduced);
-  }
-}
-
 static
 SEXP eval_filter_one(SEXP quos,
                      SEXP mask,
@@ -174,10 +129,8 @@ SEXP eval_filter_one(SEXP quos,
 
     if (filter_is_valid_lgl(res, first)) {
       filter_lgl_reduce(res, n, p_reduced);
-    } else if (Rf_inherits(res, "data.frame")) {
-      filter_df_reduce(res, n, first, i, quos, p_reduced);
     } else {
-      dplyr::stop_filter_incompatible_type(i, quos, R_NilValue, res);
+      dplyr::stop_filter_incompatible_type(i, quos, res);
     }
 
     UNPROTECT(2);

src/imports.cpp

@@ -43,10 +43,6 @@ SEXP str_as_symbol(SEXP str) {
   return rlang_api().str_as_symbol(str);
 }
 
-SEXP quo_get_expr(SEXP quo) {
-  return rlang_api().quo_get_expr(quo);
-}
-
 void env_unbind(SEXP env, SEXP sym) {
   rlang_api().env_unbind(env, sym);
 }

tests/testthat/_snaps/filter.md

@@ -92,66 +92,6 @@
       i In argument: `c(TRUE, FALSE)`.
       Caused by error:
       ! `..1` must be of size 150 or 1, not size 2.
-    Code
-      (expect_error(filter(group_by(iris, Species), data.frame(c(TRUE, FALSE)))))
-    Output
-      <error/rlang_error>
-      Error in `filter()`:
-      i In argument: `data.frame(c(TRUE, FALSE))`.
-      i In group 1: `Species = setosa`.
-      Caused by error:
-      ! `..1` must be of size 50 or 1, not size 2.
-    Code
-      (expect_error(filter(rowwise(iris), data.frame(c(TRUE, FALSE)))))
-    Output
-      <error/rlang_error>
-      Error in `filter()`:
-      i In argument: `data.frame(c(TRUE, FALSE))`.
-      i In row 1.
-      Caused by error:
-      ! `..1` must be of size 1, not size 2.
-    Code
-      (expect_error(filter(iris, data.frame(c(TRUE, FALSE)))))
-    Output
-      <error/rlang_error>
-      Error in `filter()`:
-      i In argument: `data.frame(c(TRUE, FALSE))`.
-      Caused by error:
-      ! `..1` must be of size 150 or 1, not size 2.
-    Code
-      (expect_error(filter(tibble(x = 1), c(TRUE, TRUE))))
-    Output
-      <error/rlang_error>
-      Error in `filter()`:
-      i In argument: `c(TRUE, TRUE)`.
-      Caused by error:
-      ! `..1` must be of size 1, not size 2.
-    Code
-      (expect_error(filter(group_by(iris, Species), data.frame(Sepal.Length > 3, 1:n())))
-      )
-    Condition
-      Warning:
-      Returning data frames from `filter()` expressions was deprecated in dplyr 1.0.8.
-      i Please use `if_any()` or `if_all()` instead.
-    Output
-      <error/rlang_error>
-      Error in `filter()`:
-      i In argument: `data.frame(Sepal.Length > 3, 1:n())`.
-      i In group 1: `Species = setosa`.
-      Caused by error:
-      ! `..1$X1.n..` must be a logical vector, not an integer vector.
-    Code
-      (expect_error(filter(iris, data.frame(Sepal.Length > 3, 1:n()))))
-    Condition
-      Warning:
-      Returning data frames from `filter()` expressions was deprecated in dplyr 1.0.8.
-      i Please use `if_any()` or `if_all()` instead.
-    Output
-      <error/rlang_error>
-      Error in `filter()`:
-      i In argument: `data.frame(Sepal.Length > 3, 1:n())`.
-      Caused by error:
-      ! `..1$X1.n..` must be a logical vector, not an integer vector.
     Code
       (expect_error(filter(mtcars, `_x`)))
     Output
@@ -209,24 +149,76 @@
       i In argument: `stop("{")`.
       Caused by error:
       ! {
+
+# Using data frames in `filter()` is defunct (#7758)
+
     Code
-      filter(data.frame(x = 1, y = 1), across(everything(), ~ .x > 0))
+      filter(df, across(everything(), ~ .x > 0))
     Condition
-      Warning:
-      Using `across()` in `filter()` was deprecated in dplyr 1.0.8.
-      i Please use `if_any()` or `if_all()` instead.
-    Output
-        x y
-      1 1 1
+      Error in `filter()`:
+      i In argument: `across(everything(), ~.x > 0)`.
+      Caused by error:
+      ! `..1` must be a logical vector, not a <tbl_df/tbl/data.frame> object.
+      i If you used `across()` to generate this data frame, please use `if_any()` or `if_all()` instead.
+
+---
+
     Code
-      filter(data.frame(x = 1, y = 1), data.frame(x > 0, y > 0))
+      filter(gdf, across(everything(), ~ .x > 0))
     Condition
-      Warning:
-      Returning data frames from `filter()` expressions was deprecated in dplyr 1.0.8.
-      i Please use `if_any()` or `if_all()` instead.
-    Output
-        x y
-      1 1 1
+      Error in `filter()`:
+      i In argument: `across(everything(), ~.x > 0)`.
+      i In group 1: `x = 1`.
+      Caused by error:
+      ! `..1` must be a logical vector, not a <tbl_df/tbl/data.frame> object.
+      i If you used `across()` to generate this data frame, please use `if_any()` or `if_all()` instead.
+
+---
+
+    Code
+      filter(rdf, across(everything(), ~ .x > 0))
+    Condition
+      Error in `filter()`:
+      i In argument: `across(everything(), ~.x > 0)`.
+      i In row 1.
+      Caused by error:
+      ! `..1` must be a logical vector, not a <tbl_df/tbl/data.frame> object.
+      i If you used `across()` to generate this data frame, please use `if_any()` or `if_all()` instead.
+
+---
+
+    Code
+      filter(df, tibble(x > 0, y > 0))
+    Condition
+      Error in `filter()`:
+      i In argument: `tibble(x > 0, y > 0)`.
+      Caused by error:
+      ! `..1` must be a logical vector, not a <tbl_df/tbl/data.frame> object.
+      i If you used `across()` to generate this data frame, please use `if_any()` or `if_all()` instead.
+
+---
+
+    Code
+      filter(gdf, tibble(x > 0, y > 0))
+    Condition
+      Error in `filter()`:
+      i In argument: `tibble(x > 0, y > 0)`.
+      i In group 1: `x = 1`.
+      Caused by error:
+      ! `..1` must be a logical vector, not a <tbl_df/tbl/data.frame> object.
+      i If you used `across()` to generate this data frame, please use `if_any()` or `if_all()` instead.
+
+---
+
+    Code
+      filter(rdf, tibble(x > 0, y > 0))
+    Condition
+      Error in `filter()`:
+      i In argument: `tibble(x > 0, y > 0)`.
+      i In row 1.
+      Caused by error:
+      ! `..1` must be a logical vector, not a <tbl_df/tbl/data.frame> object.
+      i If you used `across()` to generate this data frame, please use `if_any()` or `if_all()` instead.
 
 # `filter()` doesn't allow data frames with missing or empty names (#6758)