From dd0676681b577192b939371a5f8f2d43bea2748f Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Fri, 15 Mar 2024 12:13:22 -0500 Subject: [PATCH] Advance deprecation status (#1478) --- NEWS.md | 4 + R/backend-teradata.R | 2 +- R/db-sql.R | 4 +- R/src-sql.R | 4 +- R/tidyeval.R | 19 ++- R/verb-group_by.R | 12 +- R/verb-joins.R | 2 +- man/group_by.tbl_lazy.Rd | 2 +- man/partial_eval.Rd | 2 +- revdep/README.md | 23 ++-- revdep/cran.md | 8 +- revdep/failures.md | 8 +- revdep/problems.md | 153 +++++++++++-------------- tests/testthat/_snaps/tidyeval.md | 13 +++ tests/testthat/_snaps/verb-group_by.md | 9 ++ tests/testthat/test-tidyeval.R | 7 ++ tests/testthat/test-verb-group_by.R | 7 +- 17 files changed, 140 insertions(+), 139 deletions(-) create mode 100644 tests/testthat/_snaps/tidyeval.md diff --git a/NEWS.md b/NEWS.md index 8b8061992..552400b12 100644 --- a/NEWS.md +++ b/NEWS.md @@ -93,6 +93,10 @@ ## Minor improvements and bug fixes +* Deprecation status of functions deprecated in previous versions (at least + 2 years old) have been advanced. In particular, `src_sql()` is now defunct, + as is the use of `partial_eval()` with character `data`. + * Database errors now show the generated SQL, which hopefully will make it faster to track down problems (#1401). diff --git a/R/backend-teradata.R b/R/backend-teradata.R index fe2007ee4..ba17d3a45 100644 --- a/R/backend-teradata.R +++ b/R/backend-teradata.R @@ -221,7 +221,7 @@ utils::globalVariables(c("ATAN2", "SUBSTR", "DECIMAL", "WEEKNUMBER_OF_YEAR", "SU #' @export #' @rdname win_over win_rank_tdata <- function(f) { - lifecycle::deprecate_soft( + lifecycle::deprecate_warn( "2.4.0", what = "win_rank_tdata()", with = "win_rank()" diff --git a/R/db-sql.R b/R/db-sql.R index 0198b4f45..f29aa1021 100644 --- a/R/db-sql.R +++ b/R/db-sql.R @@ -823,9 +823,9 @@ sql_query_append <- function(con, ..., returning_cols = NULL) { if (is_tbl_lazy(from)) { - lifecycle::deprecate_soft( + lifecycle::deprecate_warn( when = "2.3.2", - what = "sql_query_append(from = 'must be a table identifier or an SQL query, not a lazy table.')", + what = "sql_query_append(from = 'must be a table identifier or an SQL query, not a lazy table.')" ) insert_cols <- colnames(from) diff --git a/R/src-sql.R b/R/src-sql.R index b7739843f..eadf3957f 100644 --- a/R/src-sql.R +++ b/R/src-sql.R @@ -12,13 +12,11 @@ #' @param con the connection object #' @param ... fields used by object src_sql <- function(subclass, con, ...) { - lifecycle::deprecate_warn( + lifecycle::deprecate_stop( when = "1.4.0", what = "src_sql()", always = TRUE ) - subclass <- paste0("src_", subclass) - structure(list(con = con, ...), class = c(subclass, "src_sql", "src")) } diff --git a/R/tidyeval.R b/R/tidyeval.R index 7583de080..fbc747f2a 100644 --- a/R/tidyeval.R +++ b/R/tidyeval.R @@ -51,19 +51,16 @@ #' f <- function(x) x + 1 #' partial_eval(quote(year > f(1980)), lf) #' partial_eval(quote(year > local(f(1980))), lf) -partial_eval <- function(call, data, env = caller_env(), vars = NULL, error_call) { - if (!is_null(vars)) { - lifecycle::deprecate_warn("2.1.2", "partial_eval(vars)", always = TRUE) - data <- lazy_frame(!!!rep_named(vars, list(logical()))) +partial_eval <- function(call, + data, + env = caller_env(), + vars = deprecated(), + error_call) { + if (lifecycle::is_present(vars)) { + lifecycle::deprecate_stop("2.1.2", "partial_eval(vars)") } - if (is.character(data)) { - lifecycle::deprecate_warn( - "2.1.2", - "partial_eval(data = 'must be a lazy frame')", - always = TRUE - ) - data <- lazy_frame(!!!rep_named(data, list(logical()))) + lifecycle::deprecate_stop("2.1.2", "partial_eval(data = 'must be a lazy frame')", ) } if (is_sql_literal(call)) { diff --git a/R/verb-group_by.R b/R/verb-group_by.R index 73b5391eb..5d7815112 100644 --- a/R/verb-group_by.R +++ b/R/verb-group_by.R @@ -24,17 +24,11 @@ #' group_by(g) %>% #' mutate(x2 = x / sum(x, na.rm = TRUE)) %>% #' show_query() -group_by.tbl_lazy <- function(.data, ..., .add = FALSE, add = NULL, .drop = TRUE) { +group_by.tbl_lazy <- function(.data, ..., .add = FALSE, add = deprecated(), .drop = TRUE) { dots <- partial_eval_dots(.data, ..., .named = FALSE) - if (!missing(add)) { - lifecycle::deprecate_warn( - "1.0.0", - "dplyr::group_by(add = )", - "group_by(.add = )", - always = TRUE - ) - .add <- add + if (lifecycle::is_present(add)) { + lifecycle::deprecate_stop("1.0.0", "dplyr::group_by(add)", "group_by(.add)") } check_unsupported_arg(.drop, TRUE) diff --git a/R/verb-joins.R b/R/verb-joins.R index 82d72164e..0bb0accd7 100644 --- a/R/verb-joins.R +++ b/R/verb-joins.R @@ -584,7 +584,7 @@ join_prepare_by <- function(by, user_env = caller_env(3)) { if (identical(by, character()) && is.null(sql_on)) { if (type != "cross") { - lifecycle::deprecate_soft( + lifecycle::deprecate_warn( when = "1.1.0", what = I("Using `by = character()` to perform a cross join"), with = "cross_join()", diff --git a/man/group_by.tbl_lazy.Rd b/man/group_by.tbl_lazy.Rd index e06905bcf..1c4cf1f97 100644 --- a/man/group_by.tbl_lazy.Rd +++ b/man/group_by.tbl_lazy.Rd @@ -4,7 +4,7 @@ \alias{group_by.tbl_lazy} \title{Group by one or more variables} \usage{ -\method{group_by}{tbl_lazy}(.data, ..., .add = FALSE, add = NULL, .drop = TRUE) +\method{group_by}{tbl_lazy}(.data, ..., .add = FALSE, add = deprecated(), .drop = TRUE) } \arguments{ \item{.data}{A lazy data frame backed by a database query.} diff --git a/man/partial_eval.Rd b/man/partial_eval.Rd index b45659cd6..512f14655 100644 --- a/man/partial_eval.Rd +++ b/man/partial_eval.Rd @@ -4,7 +4,7 @@ \alias{partial_eval} \title{Partially evaluate an expression.} \usage{ -partial_eval(call, data, env = caller_env(), vars = NULL, error_call) +partial_eval(call, data, env = caller_env(), vars = deprecated(), error_call) } \arguments{ \item{call}{an unevaluated expression, as produced by \code{\link[=quote]{quote()}}} diff --git a/revdep/README.md b/revdep/README.md index d0228de7b..ee85c7ae8 100644 --- a/revdep/README.md +++ b/revdep/README.md @@ -16,17 +16,16 @@ |SQLDataFrame |? | | | | |synaptome.db |? | | | | -## New problems (9) +## New problems (8) -|package |version |error |warning |note | -|:-------------------|:-------|:------|:-------|:----| -|[Andromeda](problems.md#andromeda)|0.6.5 |__+3__ | | | -|[CDMConnector](problems.md#cdmconnector)|1.3.0 |__+2__ | |1 | -|[childesr](problems.md#childesr)|0.2.3 |__+1__ | |1 | -|[CohortSurvival](problems.md#cohortsurvival)|0.3.0 |__+2__ | | | -|[diseasystore](problems.md#diseasystore)|0.1.1 |__+1__ | | | -|[DrugUtilisation](problems.md#drugutilisation)|0.5.0 |__+2__ | | | -|[IncidencePrevalence](problems.md#incidenceprevalence)|0.7.1 |__+2__ | | | -|[PatientProfiles](problems.md#patientprofiles)|0.6.1 |__+2__ | | | -|[SCDB](problems.md#scdb)|0.3 |__+3__ | | | +|package |version |error |warning |note | +|:-------------------|:-------|:--------|:-------|:----| +|[Andromeda](problems.md#andromeda)|0.6.5 |__+3__ |-1 | | +|[CDMConnector](problems.md#cdmconnector)|1.3.0 |__+2__ | |1 | +|[CohortSurvival](problems.md#cohortsurvival)|0.3.0 |__+2__ | | | +|[diseasystore](problems.md#diseasystore)|0.1.1 |__+1__ | | | +|[DrugUtilisation](problems.md#drugutilisation)|0.5.2 |__+2__ | | | +|[IncidencePrevalence](problems.md#incidenceprevalence)|0.7.1 |__+2__ | | | +|[PatientProfiles](problems.md#patientprofiles)|0.6.2 |1 __+1__ | | | +|[SCDB](problems.md#scdb)|0.3 |__+3__ | | | diff --git a/revdep/cran.md b/revdep/cran.md index be2fa4ff8..52ddbddbf 100644 --- a/revdep/cran.md +++ b/revdep/cran.md @@ -1,8 +1,8 @@ ## revdepcheck results -We checked 111 reverse dependencies (100 from CRAN + 11 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package. +We checked 112 reverse dependencies (101 from CRAN + 11 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package. - * We saw 9 new problems + * We saw 8 new problems * We failed to check 0 packages Issues with CRAN packages are summarised below. @@ -19,9 +19,6 @@ Issues with CRAN packages are summarised below. checking tests ... ERROR checking re-building of vignette outputs ... ERROR -* childesr - checking re-building of vignette outputs ... ERROR - * CohortSurvival checking tests ... ERROR checking re-building of vignette outputs ... ERROR @@ -38,7 +35,6 @@ Issues with CRAN packages are summarised below. checking re-building of vignette outputs ... ERROR * PatientProfiles - checking tests ... ERROR checking re-building of vignette outputs ... ERROR * SCDB diff --git a/revdep/failures.md b/revdep/failures.md index 998a57830..6ce3ca189 100644 --- a/revdep/failures.md +++ b/revdep/failures.md @@ -110,7 +110,7 @@ Run `revdepcheck::cloud_details(, "CompoundDb")` for more info * Version: NA * GitHub: NA * Source code: https://github.com/cran/CuratedAtlasQueryR -* Number of recursive dependencies: 197 +* Number of recursive dependencies: 196 Run `revdepcheck::cloud_details(, "CuratedAtlasQueryR")` for more info @@ -215,7 +215,7 @@ Run `revdepcheck::cloud_details(, "grasp2db")` for more info * Version: NA * GitHub: NA * Source code: https://github.com/cran/msPurity -* Number of recursive dependencies: 185 +* Number of recursive dependencies: 183 Run `revdepcheck::cloud_details(, "msPurity")` for more info @@ -320,7 +320,7 @@ Run `revdepcheck::cloud_details(, "Organism.dplyr")` for more info * Version: NA * GitHub: NA * Source code: https://github.com/cran/SQLDataFrame -* Number of recursive dependencies: 95 +* Number of recursive dependencies: 93 Run `revdepcheck::cloud_details(, "SQLDataFrame")` for more info @@ -355,7 +355,7 @@ Run `revdepcheck::cloud_details(, "SQLDataFrame")` for more info * Version: NA * GitHub: NA * Source code: https://github.com/cran/synaptome.db -* Number of recursive dependencies: 151 +* Number of recursive dependencies: 149 Run `revdepcheck::cloud_details(, "synaptome.db")` for more info diff --git a/revdep/problems.md b/revdep/problems.md index aa369d64f..94041043c 100644 --- a/revdep/problems.md +++ b/revdep/problems.md @@ -82,6 +82,33 @@ Run `revdepcheck::cloud_details(, "Andromeda")` for more info Execution halted ``` +## Newly fixed + +* checking re-building of vignette outputs ... WARNING + ``` + Error(s) in re-building vignettes: + ... + --- re-building ‘UsingAndromeda.Rmd’ using rmarkdown + A new version of TeX Live has been released. If you need to install or update any LaTeX packages, you have to upgrade TinyTeX with tinytex::reinstall_tinytex(repository = "illinois"). + + tlmgr: Local TeX Live (2023) is older than remote repository (2024). + Cross release updates are only supported with + update-tlmgr-latest(.sh/.exe) --update + See https://tug.org/texlive/upgrade.html for details. + Warning in system2("tlmgr", args, ...) : + ... + + Error: processing vignette 'UsingAndromeda.Rmd' failed with diagnostics: + LaTeX failed to compile /tmp/workdir/Andromeda/old/Andromeda.Rcheck/vign_test/Andromeda/vignettes/UsingAndromeda.tex. See https://yihui.org/tinytex/r/#debugging for debugging tips. See UsingAndromeda.log for more info. + --- failed re-building ‘UsingAndromeda.Rmd’ + + SUMMARY: processing the following file failed: + ‘UsingAndromeda.Rmd’ + + Error: Vignette re-building failed. + Execution halted + ``` + # CDMConnector
@@ -111,6 +138,7 @@ Run `revdepcheck::cloud_details(, "CDMConnector")` for more info > # * https://r-pkgs.org/tests.html > # * https://testthat.r-lib.org/reference/test_package.html#special-files ... + In addition: Warning messages: 1: Connection is garbage-collected, use dbDisconnect() to avoid this. 2: Connection is garbage-collected, use dbDisconnect() to avoid this. 3: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. @@ -118,9 +146,8 @@ Run `revdepcheck::cloud_details(, "CDMConnector")` for more info 5: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. 6: Connection is garbage-collected, use dbDisconnect() to avoid this. 7: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. + 8: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. Execution halted - Warning message: - Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. ``` * checking re-building of vignette outputs ... ERROR @@ -156,50 +183,6 @@ Run `revdepcheck::cloud_details(, "CDMConnector")` for more info 'arrow', 'CirceR', 'Capr' ``` -# childesr - -
- -* Version: 0.2.3 -* GitHub: https://github.com/langcog/childesr -* Source code: https://github.com/cran/childesr -* Date/Publication: 2022-01-27 00:00:02 UTC -* Number of recursive dependencies: 48 - -Run `revdepcheck::cloud_details(, "childesr")` for more info - -
- -## Newly broken - -* checking re-building of vignette outputs ... ERROR - ``` - Error(s) in re-building vignettes: - ... - --- re-building ‘access_childes_db.Rmd’ using rmarkdown - - Quitting from lines 101-103 [unnamed-chunk-8] (access_childes_db.Rmd) - Error: processing vignette 'access_childes_db.Rmd' failed with diagnostics: - Failed to collect lazy table. - Caused by error in `dbSendQuery()`: - ! could not run statement: Unknown column 'participant.*' in 'field list' - --- failed re-building ‘access_childes_db.Rmd’ - - SUMMARY: processing the following file failed: - ‘access_childes_db.Rmd’ - - Error: Vignette re-building failed. - Execution halted - ``` - -## In both - -* checking dependencies in R code ... NOTE - ``` - Namespace in Imports field not imported from: ‘dbplyr’ - All declared Imports should be used. - ``` - # CohortSurvival
@@ -208,7 +191,7 @@ Run `revdepcheck::cloud_details(, "childesr")` for more info * GitHub: NA * Source code: https://github.com/cran/CohortSurvival * Date/Publication: 2024-02-08 09:50:02 UTC -* Number of recursive dependencies: 140 +* Number of recursive dependencies: 114 Run `revdepcheck::cloud_details(, "CohortSurvival")` for more info @@ -311,11 +294,11 @@ Run `revdepcheck::cloud_details(, "diseasystore")` for more info
-* Version: 0.5.0 +* Version: 0.5.2 * GitHub: NA * Source code: https://github.com/cran/DrugUtilisation -* Date/Publication: 2024-02-10 18:10:02 UTC -* Number of recursive dependencies: 163 +* Date/Publication: 2024-03-06 01:00:29 UTC +* Number of recursive dependencies: 144 Run `revdepcheck::cloud_details(, "DrugUtilisation")` for more info @@ -338,12 +321,12 @@ Run `revdepcheck::cloud_details(, "DrugUtilisation")` for more info ... Backtrace: ▆ - 1. ├─testthat::expect_true(inherits(x, "omop_result")) at test-summariseTreatment.R:11:3 + 1. ├─testthat::expect_true(inherits(x, "summarised_result")) at test-summariseTreatment.R:11:3 2. │ └─testthat::quasi_label(enquo(object), label, arg = "object") 3. │ └─rlang::eval_bare(expr, quo_get_env(quo)) - 4. └─base::inherits(x, "omop_result") + 4. └─base::inherits(x, "summarised_result") - [ FAIL 13 | WARN 9 | SKIP 19 | PASS 115 ] + [ FAIL 8 | WARN 36 | SKIP 19 | PASS 139 ] Error: Test failures Execution halted ``` @@ -361,13 +344,13 @@ Run `revdepcheck::cloud_details(, "DrugUtilisation")` for more info Warning: Connection is garbage-collected, use dbDisconnect() to avoid this. Warning: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. ... - Warning: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. + --- finished re-building ‘routePatternDose.Rmd’ + Warning: Connection is garbage-collected, use dbDisconnect() to avoid this. Warning: Database is garbage-collected, use dbDisconnect(con, shutdown=TRUE) or duckdb::duckdb_shutdown(drv) to avoid this. SUMMARY: processing the following files failed: ‘a01_introCreateCohort.Rmd’ ‘a03_addIndications-example.Rmd’ - ‘a04_addDrugInfo.Rmd’ ‘a06_treatmentSummary.Rmd’ - ‘routePatternDose.Rmd’ + ‘a06_treatmentSummary.Rmd’ Error: Vignette re-building failed. Execution halted @@ -443,11 +426,11 @@ Run `revdepcheck::cloud_details(, "IncidencePrevalence")` for more info
-* Version: 0.6.1 +* Version: 0.6.2 * GitHub: NA * Source code: https://github.com/cran/PatientProfiles -* Date/Publication: 2024-02-21 23:30:07 UTC -* Number of recursive dependencies: 174 +* Date/Publication: 2024-03-05 18:50:03 UTC +* Number of recursive dependencies: 156 Run `revdepcheck::cloud_details(, "PatientProfiles")` for more info @@ -455,31 +438,6 @@ Run `revdepcheck::cloud_details(, "PatientProfiles")` for more info ## Newly broken -* checking tests ... ERROR - ``` - Running ‘spelling.R’ - Running ‘testthat.R’ - Running the tests in ‘tests/testthat.R’ failed. - Complete output: - > # This file is part of the standard setup for testthat. - > # It is recommended that you do not modify it. - > # - > # Where should you do additional test configuration? - > # Learn more about the roles of various files in: - > # * https://r-pkgs.org/tests.html - ... - 26. │ └─dbplyr (local) FUN(X[[i]], ...) - 27. │ ├─dbplyr::escape(eval_tidy(x, mask), con = con) - 28. │ └─rlang::eval_tidy(x, mask) - 29. └─CDMConnector::datediff - 30. └─cli::cli_abort(...) - 31. └─rlang::abort(...) - - [ FAIL 65 | WARN 0 | SKIP 1 | PASS 201 ] - Error: Test failures - Execution halted - ``` - * checking re-building of vignette outputs ... ERROR ``` Error(s) in re-building vignettes: @@ -505,6 +463,33 @@ Run `revdepcheck::cloud_details(, "PatientProfiles")` for more info Execution halted ``` +## In both + +* checking tests ... ERROR + ``` + Running ‘spelling.R’ + Running ‘testthat.R’ + Running the tests in ‘tests/testthat.R’ failed. + Complete output: + > # This file is part of the standard setup for testthat. + > # It is recommended that you do not modify it. + > # + > # Where should you do additional test configuration? + > # Learn more about the roles of various files in: + > # * https://r-pkgs.org/tests.html + ... + 26. │ └─dbplyr (local) FUN(X[[i]], ...) + 27. │ ├─dbplyr::escape(eval_tidy(x, mask), con = con) + 28. │ └─rlang::eval_tidy(x, mask) + 29. └─CDMConnector::datediff + 30. └─cli::cli_abort(...) + 31. └─rlang::abort(...) + + [ FAIL 65 | WARN 21 | SKIP 1 | PASS 200 ] + Error: Test failures + Execution halted + ``` + # SCDB
diff --git a/tests/testthat/_snaps/tidyeval.md b/tests/testthat/_snaps/tidyeval.md new file mode 100644 index 000000000..0c15cfeb9 --- /dev/null +++ b/tests/testthat/_snaps/tidyeval.md @@ -0,0 +1,13 @@ +# old arguments are defunct + + Code + partial_eval(quote(x), vars = c("x", "y")) + Condition + Error: + ! The `vars` argument of `partial_eval()` was deprecated in dbplyr 2.1.2 and is now defunct. + Code + partial_eval(quote(x), data = c("x", "y")) + Condition + Error: + ! The `data` argument of `partial_eval()` must be a lazy frame as of dbplyr 2.1.2. + diff --git a/tests/testthat/_snaps/verb-group_by.md b/tests/testthat/_snaps/verb-group_by.md index 53e922e6a..cc425afa0 100644 --- a/tests/testthat/_snaps/verb-group_by.md +++ b/tests/testthat/_snaps/verb-group_by.md @@ -1,3 +1,12 @@ +# errors about add argument + + Code + gf <- mf %>% group_by(x) %>% group_by(y, add = TRUE) + Condition + Error: + ! The `add` argument of `group_by()` was deprecated in dplyr 1.0.0 and is now defunct. + i Please use the `.add` argument instead. + # errors for .drop = FALSE Code diff --git a/tests/testthat/test-tidyeval.R b/tests/testthat/test-tidyeval.R index 48c15a519..9a45cbd01 100644 --- a/tests/testthat/test-tidyeval.R +++ b/tests/testthat/test-tidyeval.R @@ -73,3 +73,10 @@ test_that("fails with multi-classes", { x <- structure(list(), class = c('a', 'b')) expect_error(partial_eval(x, lf), "Unknown input type", fixed = TRUE) }) + +test_that("old arguments are defunct", { + expect_snapshot(error = TRUE, { + partial_eval(quote(x), vars = c("x", "y")) + partial_eval(quote(x), data = c("x", "y")) + }) +}) diff --git a/tests/testthat/test-verb-group_by.R b/tests/testthat/test-verb-group_by.R index 2c5ad5d43..b03f96131 100644 --- a/tests/testthat/test-verb-group_by.R +++ b/tests/testthat/test-verb-group_by.R @@ -7,13 +7,12 @@ test_that("group_by with .add = TRUE adds groups", { expect_equal(group_vars(gf2), c("x", "y")) }) -test_that("warns about add argument ", { +test_that("errors about add argument ", { mf <- memdb_frame(x = 1:3, y = 1:3) - expect_warning( + expect_snapshot( gf <- mf %>% group_by(x) %>% group_by(y, add = TRUE), - "deprecated" + error = TRUE ) - expect_equal(group_vars(gf), c("x", "y")) }) test_that("errors for .drop = FALSE", {