diff --git a/R/data-table.R b/R/data-table.R index 5ebf67e37..72edfd74e 100644 --- a/R/data-table.R +++ b/R/data-table.R @@ -2,16 +2,15 @@ #' #' @description #' -#' `use_data_table` facilitates importing the data.table package by handling -#' up-front some common set-up tasks for using it in your package. -#' -#' This function does two main things: -#' -#' 1. Import the entire data.table namespace (with `@import`). -#' -#' 2. Block the usage of data.table as a dependency (`DESCRIPTION` field -#' `Depends`); `data.table` should be used as an _imported_ or _suggested_ -#' package only. See this +#' `use_data_table()` imports the `data.table()` function from the data.table +#' package, as well as several important symbols: `:=`, `.SD`, `.BY`, `.N`, +#' `.I`, `.GRP`, `.NGRP`, `.EACHI`. This is a minimal setup to get `data.table`s +#' working with your package. See the [importing +#' data.table](https://rdatatable.gitlab.io/data.table/articles/datatable-importing.html) +#' vignette for other strategies. In addition to importing these function, +#' `use_data_table()` also blocks the usage of data.table in the `Depends` field +#' of the `DESCRIPTION` file; `data.table` should be used as an _imported_ or +#' _suggested_ package only. See this #' [discussion](https://github.com/Rdatatable/data.table/issues/3076). #' @export use_data_table <- function() { @@ -21,10 +20,12 @@ use_data_table <- function() { deps <- desc::desc_get_deps(".") if (any(deps$type == "Depends" & deps$package == "data.table")) { ui_warn("data.table should be in Imports or Suggests, not Depends") + ui_done("Deleting data.table from {ui_field('Depends')}") desc::desc_del_dep("data.table", "Depends", file = proj_get()) } - use_dependency("data.table", "Imports") - new <- use_template("data-table.R", "R/utils-data-table.R") - ui_todo("Run {ui_code('devtools::document()')}") - invisible(new) + + use_import_from( + "data.table", + c("data.table", ":=", ".SD", ".BY", ".N", ".I", ".GRP", ".NGRP", ".EACHI") + ) } diff --git a/R/pipe.R b/R/pipe.R index 804e89a41..d59436c64 100644 --- a/R/pipe.R +++ b/R/pipe.R @@ -22,26 +22,11 @@ use_pipe <- function(export = TRUE) { check_is_package("use_pipe()") check_uses_roxygen("use_pipe()") - use_dependency("magrittr", "Imports") - if (export) { + use_dependency("magrittr", "Imports") use_template("pipe.R", "R/utils-pipe.R") && roxygen_remind() return(invisible(TRUE)) } - if (has_package_doc()) { - roxygen_ns_append("@importFrom magrittr %>%") && roxygen_remind() - return(invisible(TRUE)) - } - - ui_todo( - "Copy and paste this line into some roxygen header, then run \\ - {ui_code('devtools::document()')}:" - ) - ui_code_block("#' @importFrom magrittr %>%", copy = FALSE) - ui_todo( - "Alternative recommendation: call {ui_code('use_package_doc()')}, then \\ - call {ui_code('use_pipe()')} again." - ) - invisible(FALSE) + use_import_from("magrittr", "%>%") } diff --git a/R/tibble.R b/R/tibble.R index 71163ef8b..2e127a203 100644 --- a/R/tibble.R +++ b/R/tibble.R @@ -31,9 +31,10 @@ use_tibble <- function() { check_is_package("use_tibble()") check_uses_roxygen("use_tibble()") - use_dependency("tibble", "Imports") - roxygen_ns_append("@importFrom tibble tibble") && roxygen_remind() + created <- use_import_from("tibble", "tibble") ui_todo("Document a returned tibble like so:") ui_code_block("#' @return a [tibble][tibble::tibble-package]", copy = FALSE) + + invisible(created) } diff --git a/R/use_import_from.R b/R/use_import_from.R index eac7c948a..d31a702e8 100644 --- a/R/use_import_from.R +++ b/R/use_import_from.R @@ -27,9 +27,9 @@ use_import_from <- function(package, fun, load = is_interactive()) { } check_is_package("use_import_from()") check_uses_roxygen("use_import_from()") + check_installed(package) check_has_package_doc("use_import_from()") - - purrr::walk2(package, fun, check_fun_exists) + check_functions_exist(package, fun) use_dependency(package, "Imports") changed <- roxygen_ns_append(glue("@importFrom {package} {fun}")) @@ -41,6 +41,10 @@ use_import_from <- function(package, fun, load = is_interactive()) { invisible(changed) } +check_functions_exist <- function(package, fun) { + purrr::walk2(package, fun, check_fun_exists) +} + check_fun_exists <- function(package, fun) { if (exists(fun, envir = asNamespace(package))) { return() diff --git a/inst/templates/data-table.R b/inst/templates/data-table.R deleted file mode 100644 index d2f29646c..000000000 --- a/inst/templates/data-table.R +++ /dev/null @@ -1,12 +0,0 @@ -# data.table is generally careful to minimize the scope for namespace -# conflicts (i.e., functions with the same name as in other packages); -# a more conservative approach using @importFrom should be careful to -# import any needed data.table special symbols as well, e.g., if you -# run DT[ , .N, by='grp'] in your package, you'll need to add -# @importFrom data.table .N to prevent the NOTE from R CMD check. -# See ?data.table::`special-symbols` for the list of such symbols -# data.table defines; see the 'Importing data.table' vignette for more -# advice (vignette('datatable-importing', 'data.table')). -# -#' @import data.table -NULL diff --git a/man/use_data_table.Rd b/man/use_data_table.Rd index 19c8ed632..865bb2c2d 100644 --- a/man/use_data_table.Rd +++ b/man/use_data_table.Rd @@ -7,15 +7,13 @@ use_data_table() } \description{ -\code{use_data_table} facilitates importing the data.table package by handling -up-front some common set-up tasks for using it in your package. - -This function does two main things: -\enumerate{ -\item Import the entire data.table namespace (with \verb{@import}). -\item Block the usage of data.table as a dependency (\code{DESCRIPTION} field -\code{Depends}); \code{data.table} should be used as an \emph{imported} or \emph{suggested} -package only. See this +\code{use_data_table()} imports the \code{data.table()} function from the data.table +package, as well as several important symbols: \verb{:=}, \code{.SD}, \code{.BY}, \code{.N}, +\code{.I}, \code{.GRP}, \code{.NGRP}, \code{.EACHI}. This is a minimal setup to get \code{data.table}s +working with your package. See the \href{https://rdatatable.gitlab.io/data.table/articles/datatable-importing.html}{importing data.table} +vignette for other strategies. In addition to importing these function, +\code{use_data_table()} also blocks the usage of data.table in the \code{Depends} field +of the \code{DESCRIPTION} file; \code{data.table} should be used as an \emph{imported} or +\emph{suggested} package only. See this \href{https://github.com/Rdatatable/data.table/issues/3076}{discussion}. } -} diff --git a/tests/testthat/_snaps/data-table.md b/tests/testthat/_snaps/data-table.md new file mode 100644 index 000000000..f1aa70eaa --- /dev/null +++ b/tests/testthat/_snaps/data-table.md @@ -0,0 +1,30 @@ +# use_data_table() Imports data.table + + Code + roxygen_ns_show() + Output + [1] "#' @importFrom data.table .BY" + [2] "#' @importFrom data.table .EACHI" + [3] "#' @importFrom data.table .GRP" + [4] "#' @importFrom data.table .I" + [5] "#' @importFrom data.table .N" + [6] "#' @importFrom data.table .NGRP" + [7] "#' @importFrom data.table .SD" + [8] "#' @importFrom data.table :=" + [9] "#' @importFrom data.table data.table" + +# use_data_table() blocks use of Depends + + Code + roxygen_ns_show() + Output + [1] "#' @importFrom data.table .BY" + [2] "#' @importFrom data.table .EACHI" + [3] "#' @importFrom data.table .GRP" + [4] "#' @importFrom data.table .I" + [5] "#' @importFrom data.table .N" + [6] "#' @importFrom data.table .NGRP" + [7] "#' @importFrom data.table .SD" + [8] "#' @importFrom data.table :=" + [9] "#' @importFrom data.table data.table" + diff --git a/tests/testthat/_snaps/pipe.md b/tests/testthat/_snaps/pipe.md deleted file mode 100644 index 749e3a636..000000000 --- a/tests/testthat/_snaps/pipe.md +++ /dev/null @@ -1,10 +0,0 @@ -# use_pipe(export = FALSE) gives advice if no package doc - - Code - use_pipe(export = FALSE) - Message - v Adding 'magrittr' to Imports field in DESCRIPTION - * Copy and paste this line into some roxygen header, then run `devtools::document()`: - #' @importFrom magrittr %>% - * Alternative recommendation: call `use_package_doc()`, then call `use_pipe()` again. - diff --git a/tests/testthat/_snaps/tibble.md b/tests/testthat/_snaps/tibble.md index 3b9f3c417..e515c1705 100644 --- a/tests/testthat/_snaps/tibble.md +++ b/tests/testthat/_snaps/tibble.md @@ -4,11 +4,7 @@ use_tibble() Message v Adding 'tibble' to Imports field in DESCRIPTION - * Copy and paste the following lines into 'R/mypackage-package.R': - ## usethis namespace: start - #' @importFrom tibble tibble - ## usethis namespace: end - NULL + v Adding '@importFrom tibble tibble' to 'R/mypackage-package.R' * Document a returned tibble like so: #' @return a [tibble][tibble::tibble-package] diff --git a/tests/testthat/test-data-table.R b/tests/testthat/test-data-table.R index c16167c8b..81438ff15 100644 --- a/tests/testthat/test-data-table.R +++ b/tests/testthat/test-data-table.R @@ -5,11 +5,32 @@ test_that("use_data_table() requires a package", { test_that("use_data_table() Imports data.table", { create_local_package() + use_package_doc() with_mock( check_installed = function(pkg) TRUE, + roxygen_update_ns = function(...) NULL, + check_functions_exist = function(...) TRUE, use_data_table() ) + + expect_match(desc::desc_get("Imports"), "data.table") + expect_snapshot(roxygen_ns_show()) +}) + +test_that("use_data_table() blocks use of Depends", { + create_local_package() + use_package_doc() + desc::desc_set_dep("data.table", "Depends") + with_mock( + check_installed = function(pkg) TRUE, + roxygen_update_ns = function(...) NULL, + check_functions_exist = function(...) TRUE, + expect_warning( + use_data_table(), + "data.table should be in Imports or Suggests, not Depends" + ) + ) + expect_match(desc::desc_get("Imports"), "data.table") - datatable_doc <- read_utf8(proj_path("R", "utils-data-table.R")) - expect_match(datatable_doc, "#' @import data.table", all = FALSE) + expect_snapshot(roxygen_ns_show()) }) diff --git a/tests/testthat/test-pipe.R b/tests/testthat/test-pipe.R index 8f29e3588..0e63b6fa0 100644 --- a/tests/testthat/test-pipe.R +++ b/tests/testthat/test-pipe.R @@ -19,9 +19,3 @@ test_that("use_pipe(export = FALSE) adds roxygen to package doc", { expect_match(package_doc, "#' @importFrom magrittr %>%", all = FALSE) }) -test_that("use_pipe(export = FALSE) gives advice if no package doc", { - create_local_package() - withr::local_options(list(usethis.quiet = FALSE)) - expect_snapshot(use_pipe(export = FALSE)) - expect_match(desc::desc_get("Imports", proj_get()), "magrittr") -}) diff --git a/tests/testthat/test-tibble.R b/tests/testthat/test-tibble.R index 3847774cc..a010b4ce5 100644 --- a/tests/testthat/test-tibble.R +++ b/tests/testthat/test-tibble.R @@ -6,6 +6,14 @@ test_that("use_tibble() requires a package", { test_that("use_tibble() Imports tibble", { create_local_package(path_temp("mypackage")) withr::local_options(list(usethis.quiet = FALSE)) - expect_snapshot(use_tibble()) + ui_silence(use_package_doc()) + + with_mock( + check_installed = function(pkg) TRUE, + roxygen_update_ns = function(...) NULL, + check_functions_exist = function(...) TRUE, + expect_snapshot(use_tibble()) + ) + expect_match(desc::desc_get("Imports", proj_get()), "tibble") })