Skip to content

Commit 6493816

Browse files
authored
Use lifecycle tooling to deprecate wt = n() and add tests (#7752)
1 parent 2a52da0 commit 6493816

File tree

3 files changed

+128
-21
lines changed

3 files changed

+128
-21
lines changed

R/count-tally.R

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,8 @@ count.data.frame <- function(
102102
out <- x
103103
}
104104

105-
out <- tally(out, wt = !!enquo(wt), sort = sort, name = name)
105+
wt <- compat_wt(enquo(wt))
106+
out <- tally(out, wt = !!wt, sort = sort, name = name)
106107

107108
# Ensure grouping is transient
108109
out <- dplyr_reconstruct(out, x)
@@ -122,7 +123,8 @@ tally.data.frame <- function(x, wt = NULL, sort = FALSE, name = NULL) {
122123

123124
dplyr_local_error_call()
124125

125-
n <- tally_n(x, {{ wt }})
126+
wt <- compat_wt(enquo(wt))
127+
n <- tally_n(x, wt)
126128

127129
local_options(dplyr.summarise.inform = FALSE)
128130
out <- summarise(x, !!name := !!n)
@@ -193,7 +195,8 @@ add_count_impl <- function(
193195
sort = FALSE,
194196
name = NULL,
195197
.drop = deprecated(),
196-
error_call = caller_env()
198+
error_call = caller_env(),
199+
user_env = caller_env(2)
197200
) {
198201
if (!is_missing(.drop)) {
199202
lifecycle::deprecate_warn("1.0.0", "add_count(.drop = )", always = TRUE)
@@ -207,7 +210,8 @@ add_count_impl <- function(
207210
out <- x
208211
}
209212

210-
add_tally(out, wt = {{ wt }}, sort = sort, name = name)
213+
wt <- compat_wt(enquo(wt), env = error_call, user_env = user_env)
214+
add_tally(out, wt = !!wt, sort = sort, name = name)
211215
}
212216

213217
#' @rdname count
@@ -217,7 +221,8 @@ add_tally <- function(x, wt = NULL, sort = FALSE, name = NULL) {
217221

218222
dplyr_local_error_call()
219223

220-
n <- tally_n(x, {{ wt }})
224+
wt <- compat_wt(enquo(wt))
225+
n <- tally_n(x, wt)
221226
out <- mutate(x, !!name := !!n)
222227

223228
if (sort) {
@@ -230,24 +235,31 @@ add_tally <- function(x, wt = NULL, sort = FALSE, name = NULL) {
230235
# Helpers -----------------------------------------------------------------
231236

232237
tally_n <- function(x, wt) {
233-
wt <- enquo(wt)
234-
235-
if (is_call(quo_get_expr(wt), "n", n = 0)) {
236-
# Provided only by dplyr 1.0.0. See #5349 for discussion.
237-
warn(c(
238-
"`wt = n()` is deprecated",
239-
i = "You can now omit the `wt` argument"
240-
))
241-
wt <- quo(NULL)
242-
}
243-
244238
if (quo_is_null(wt)) {
245239
expr(n())
246240
} else {
247241
expr(sum(!!wt, na.rm = TRUE))
248242
}
249243
}
250244

245+
compat_wt <- function(wt, env = caller_env(), user_env = caller_env(2)) {
246+
if (!is_call(quo_get_expr(wt), "n", n = 0)) {
247+
return(wt)
248+
}
249+
250+
# Provided only by dplyr 1.0.0. See #5349 for discussion.
251+
lifecycle::deprecate_warn(
252+
when = "1.0.1",
253+
what = I("`wt = n()`"),
254+
details = "You can now omit the `wt` argument.",
255+
env = env,
256+
user_env = user_env,
257+
always = TRUE
258+
)
259+
260+
quo(NULL)
261+
}
262+
251263
check_n_name <- function(
252264
name,
253265
vars,

tests/testthat/_snaps/count-tally.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,24 @@
5959
Caused by error in `1 + ""`:
6060
! non-numeric argument to binary operator
6161

62+
# count() `wt = n()` is deprecated
63+
64+
Code
65+
count(df, a, wt = n())
66+
Condition
67+
Warning:
68+
`wt = n()` was deprecated in dplyr 1.0.1.
69+
i You can now omit the `wt` argument.
70+
Output
71+
# A tibble: 5 x 2
72+
a n
73+
<int> <int>
74+
1 1 1
75+
2 2 1
76+
3 3 1
77+
4 4 1
78+
5 5 1
79+
6280
# tally() owns errors (#6139)
6381

6482
Code
@@ -70,6 +88,38 @@
7088
Caused by error in `1 + ""`:
7189
! non-numeric argument to binary operator
7290

91+
# tally() `wt = n()` is deprecated
92+
93+
Code
94+
tally(df, wt = n())
95+
Condition
96+
Warning:
97+
`wt = n()` was deprecated in dplyr 1.0.1.
98+
i You can now omit the `wt` argument.
99+
Output
100+
# A tibble: 1 x 1
101+
n
102+
<int>
103+
1 5
104+
105+
# add_count() `wt = n()` is deprecated
106+
107+
Code
108+
add_count(df, a, wt = n())
109+
Condition
110+
Warning:
111+
`wt = n()` was deprecated in dplyr 1.0.1.
112+
i You can now omit the `wt` argument.
113+
Output
114+
# A tibble: 5 x 2
115+
a n
116+
<int> <int>
117+
1 1 1
118+
2 2 1
119+
3 3 1
120+
4 4 1
121+
5 5 1
122+
73123
# add_count() owns errors (#6139)
74124

75125
Code
@@ -100,3 +150,21 @@
100150
Caused by error in `1 + ""`:
101151
! non-numeric argument to binary operator
102152

153+
# add_tally() `wt = n()` is deprecated
154+
155+
Code
156+
add_tally(df, wt = n())
157+
Condition
158+
Warning:
159+
`wt = n()` was deprecated in dplyr 1.0.1.
160+
i You can now omit the `wt` argument.
161+
Output
162+
# A tibble: 5 x 2
163+
a n
164+
<int> <int>
165+
1 1 5
166+
2 2 5
167+
3 3 5
168+
4 4 5
169+
5 5 5
170+

tests/testthat/test-count-tally.R

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,18 +123,21 @@ test_that("can only explicitly chain together multiple tallies", {
123123
})
124124
})
125125

126-
test_that("wt = n() is deprecated", {
127-
df <- data.frame(x = 1:3)
128-
expect_warning(count(df, wt = n()), "`wt = n()`", fixed = TRUE)
129-
})
130-
131126
test_that("count() owns errors (#6139)", {
132127
expect_snapshot({
133128
(expect_error(count(mtcars, new = 1 + "")))
134129
(expect_error(count(mtcars, wt = 1 + "")))
135130
})
136131
})
137132

133+
test_that("count() `wt = n()` is deprecated", {
134+
df <- tibble(a = 1:5)
135+
136+
expect_snapshot({
137+
count(df, a, wt = n())
138+
})
139+
})
140+
138141
# tally -------------------------------------------------------------------
139142

140143
test_that("tally can sort output", {
@@ -161,6 +164,14 @@ test_that("tally() owns errors (#6139)", {
161164
})
162165
})
163166

167+
test_that("tally() `wt = n()` is deprecated", {
168+
df <- tibble(a = 1:5)
169+
170+
expect_snapshot({
171+
tally(df, wt = n())
172+
})
173+
})
174+
164175
# add_count ---------------------------------------------------------------
165176

166177
test_that("add_count preserves grouping", {
@@ -178,6 +189,14 @@ test_that(".drop is deprecated", {
178189
expect_warning(out <- add_count(df, f, .drop = FALSE), "deprecated")
179190
})
180191

192+
test_that("add_count() `wt = n()` is deprecated", {
193+
df <- tibble(a = 1:5)
194+
195+
expect_snapshot({
196+
add_count(df, a, wt = n())
197+
})
198+
})
199+
181200
test_that("add_count() owns errors (#6139)", {
182201
expect_snapshot({
183202
(expect_error(add_count(mtcars, new = 1 + "")))
@@ -215,3 +234,11 @@ test_that("add_tally() owns errors (#6139)", {
215234
(expect_error(add_tally(mtcars, wt = 1 + "")))
216235
})
217236
})
237+
238+
test_that("add_tally() `wt = n()` is deprecated", {
239+
df <- tibble(a = 1:5)
240+
241+
expect_snapshot({
242+
add_tally(df, wt = n())
243+
})
244+
})

0 commit comments

Comments
 (0)