diff --git a/NEWS.md b/NEWS.md index 903f3f442..2fa13db2f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,8 @@ # dbplyr (development version) * All old lazy eval shims have been removed. +* `median()` now takes a new argument `na.rm` for consistency with `mean()`, + but still does always remove any missing values (@lorenzwalthert, #483). # dbplyr 1.4.4 diff --git a/R/translate-sql-quantile.R b/R/translate-sql-quantile.R index c7c438921..15b9bbfcb 100644 --- a/R/translate-sql-quantile.R +++ b/R/translate-sql-quantile.R @@ -29,9 +29,10 @@ sql_median <- function(f, style = c("infix", "ordered"), window = FALSE) { + warned <- FALSE quantile <- sql_quantile(f, style = style, window = window) - - function(x) { + function(x, na.rm = FALSE) { + warned <<- check_na_rm(f, na.rm, warned) quantile(x, 0.5) } } diff --git a/tests/testthat/test-translate-sql-quantile.R b/tests/testthat/test-translate-sql-quantile.R index 820957f87..1a0bbd03b 100644 --- a/tests/testthat/test-translate-sql-quantile.R +++ b/tests/testthat/test-translate-sql-quantile.R @@ -4,13 +4,23 @@ test_that("quantile and median don't change without warning", { reg <- list( quantile = translate_sql(quantile(x, 0.75), window = FALSE), quantile_win = translate_sql(quantile(x, 0.75), vars_group = "g"), - median = translate_sql(median(x), window = FALSE), - median_win = translate_sql(median(x), vars_group = "g") + median = translate_sql(median(x, na.rm = TRUE), window = FALSE), + median_win = translate_sql(median(x, na.rm = TRUE), vars_group = "g") ) expect_known_output(print(reg), test_path("sql/backend-quantile.sql")) }) + +test_that("median functions warn once if na.rm = FALSE", { + old <- set_current_con(simulate_dbi()) + on.exit(set_current_con(old)) + + expect_warning(translate_sql(median("x")), "Missing values") + expect_warning(translate_sql(median("x")), NA) + expect_warning(translate_sql(median("x", na.rm = TRUE)), NA) +}) + test_that("checks for invalid probs", { expect_error(check_probs("a"), "numeric") expect_error(check_probs(1:3), "single value")