Skip to content

Commit

Permalink
Deprecate usage of 1 column matrices in filter() (tidyverse#6472)
Browse files Browse the repository at this point in the history
* Silence `summarise()` message

* Deprecate usage of 1 column matrices in `filter()`

* NEWS bullet
  • Loading branch information
DavisVaughan committed Sep 21, 2022
1 parent 4560f39 commit 15ab8b2
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 8 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# dplyr (development version)

* Using 1 column matrices in `filter()` is now deprecated (#6091).

* Warnings emitted inside `mutate()` and variants are now collected and stashed
away. Run the new `last_dplyr_warnings()` function to see the warnings emitted
within dplyr verbs during the last top-level command.
Expand Down
12 changes: 12 additions & 0 deletions R/filter.R
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,9 @@ filter_eval <- function(dots, mask, error_call = caller_env()) {
abort(bullets, call = error_call, parent = skip_internal_condition(e))

},
`dplyr:::signal_filter_one_column_matrix` = function(e) {
warn_filter_one_column_matrix(call = error_call)
},
`dplyr:::signal_filter_across` = function(e) {
warn_filter_across(call = error_call)
},
Expand Down Expand Up @@ -229,6 +232,15 @@ filter_bullets.default <- function(cnd, ...) {
)
}

warn_filter_one_column_matrix <- function(call) {
lifecycle::deprecate_warn(
when = "1.1.0",
what = I("Using one column matrices in `filter()`"),
with = I("one dimensional logical vectors"),
env = call
)
}

warn_filter_across <- function(call) {
lifecycle::deprecate_warn(
when = "1.0.8",
Expand Down
7 changes: 5 additions & 2 deletions src/filter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ void signal_filter(const char* cls) {
UNPROTECT(2);
}
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");
}
Expand Down Expand Up @@ -91,8 +95,7 @@ bool filter_is_valid_lgl(SEXP x, bool first) {
// 1 column matrix. We allow these with a warning that this will be
// deprecated in the future.
if (first) {
// TODO: Warn on 1 column matrices
// dplyr::signal_filter_one_column_matrix();
dplyr::signal_filter_one_column_matrix();
}
UNPROTECT(1);
return true;
Expand Down
18 changes: 18 additions & 0 deletions tests/testthat/_snaps/filter.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
# filter() allows matrices with 1 column with a deprecation warning (#6091)

Code
out <- filter(df, matrix(c(TRUE, FALSE), nrow = 2))
Condition
Warning:
Using one column matrices in `filter()` was deprecated in dplyr 1.1.0.
Please use one dimensional logical vectors instead.

---

Code
out <- filter(gdf, matrix(c(TRUE, FALSE), nrow = 2))
Condition
Warning:
Using one column matrices in `filter()` was deprecated in dplyr 1.1.0.
Please use one dimensional logical vectors instead.

# filter() disallows matrices with >1 column

Code
Expand Down
21 changes: 15 additions & 6 deletions tests/testthat/test-filter.R
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ test_that("$ does not end call traversing. #502", {
# Do some aggregation
trial_outcomes <- d %>%
group_by(Subject, TrialNo) %>%
summarise(MeanOutcome = mean(Outcome))
summarise(MeanOutcome = mean(Outcome), .groups = "drop")

left <- filter(trial_outcomes, MeanOutcome < analysis_opts$min_outcome)
right <- filter(trial_outcomes, analysis_opts$min_outcome > MeanOutcome)
Expand Down Expand Up @@ -381,11 +381,20 @@ test_that("filter() allows 1 dimension arrays", {
expect_identical(filter(df, x), df[c(1, 3),])
})

test_that("filter() allowing matrices with 1 column", {
out <- expect_warning(
filter(data.frame(x = 1:2), matrix(c(TRUE, FALSE), nrow = 2)), NA
)
expect_identical(out, data.frame(x = 1L))
test_that("filter() allows matrices with 1 column with a deprecation warning (#6091)", {
df <- tibble(x = 1:2)
expect_snapshot({
out <- filter(df, matrix(c(TRUE, FALSE), nrow = 2))
})
expect_identical(out, tibble(x = 1L))

# Only warns once when grouped
df <- tibble(x = c(1, 1, 2, 2))
gdf <- group_by(df, x)
expect_snapshot({
out <- filter(gdf, matrix(c(TRUE, FALSE), nrow = 2))
})
expect_identical(out, group_by(tibble(x = c(1, 2)), x))
})

test_that("filter() disallows matrices with >1 column", {
Expand Down

0 comments on commit 15ab8b2

Please sign in to comment.