diff --git a/NEWS.md b/NEWS.md index 4b67e40..a78aa11 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # plume (development version) +* The `by` parameter in `$set_*()` methods is now deprecated in favour of `.by` (#41). + * `Plume` gains a new method `$set_main_contributors()` that allows you to force one or more contributors to appear first in the contribution list for any given role. `Plume`'s contructor also regains the parameter `by` to set the default `by`/`.by` value used in all `set_*()` methods. * Updated the `encyclopedists` and `encyclopedists_fr` data to use the new role column system (#39). diff --git a/R/status-setter.R b/R/status-setter.R index 1aa4b66..6c7f6fb 100644 --- a/R/status-setter.R +++ b/R/status-setter.R @@ -21,19 +21,28 @@ StatusSetter <- R6Class( #' @param ... One or more unquoted expressions separated by commas. #' Expressions matching values in the column defined by `by` determine #' corresponding authors. Matching of values is case-insensitive. - #' @param by Variable used to set corresponding authors. By default, uses + #' @param .by Variable used to set corresponding authors. By default, uses #' authors' id. + #' @param by `r lifecycle::badge("deprecated")` + #' + #' Please use the `.by` parameter instead. #' @return The class instance. - set_corresponding_authors = function(..., by) { - private$set_status("corresponding", ..., by = by) + set_corresponding_authors = function(..., .by, by = deprecated()) { + private$set_status("corresponding", ..., .by = .by, by = by) } ), private = list( by = NULL, - set_status = function(col, ..., by) { - by <- private$snatch_by() + set_status = function(col, ..., .by, by) { + if (lifecycle::is_present(by)) { + call <- if (col == "corresponding") "corresponding_author" else col + call <- glue("set_{call}") + lifecycle::deprecate_warn("0.2.0", glue("{call}(by)"), glue("{call}(.by)")) + .by <- by + } + by <- private$process_by(.by) binder$bind(private$plume[[by]]) private$plume <- mutate( private$plume, @@ -42,14 +51,11 @@ StatusSetter <- R6Class( invisible(self) }, - snatch_by = function() { - env <- caller_env(2L) - by <- env$by %||% env$.by + process_by = function(by) { if (missing(by)) { by <- private$by } else { - arg <- if (is.null(env$by)) ".by" else "by" - check_string(by, allow_empty = FALSE, arg = arg) + check_string(by, allow_empty = FALSE, arg = ".by") } private$check_col(by) by @@ -83,7 +89,7 @@ StatusSetterPlume <- R6Class( private = list( set_ranks = function(..., .roles, .by) { check_character(.roles, allow_duplicates = FALSE) - by <- private$snatch_by() + by <- private$process_by(.by) vars <- private$pick("role", "contributor_rank", squash = FALSE) dots <- collect_dots(...) if (!is_named(dots)) { @@ -107,22 +113,28 @@ StatusSetterPlumeQuarto <- R6Class( #' @param ... One or more unquoted expressions separated by commas. #' Expressions matching values in the column defined by `by` determine #' equal contributors. Matching of values is case-insensitive. - #' @param by Variable used to specify which authors are equal contributors. + #' @param .by Variable used to specify which authors are equal contributors. #' By default, uses authors' id. + #' @param by `r lifecycle::badge("deprecated")` + #' + #' Please use the `.by` parameter instead. #' @return The class instance. - set_equal_contributor = function(..., by) { - private$set_status("equal_contributor", ..., by = by) + set_equal_contributor = function(..., .by, by = deprecated()) { + private$set_status("equal_contributor", ..., .by = .by, by = by) }, #' @description Set deceased authors. #' @param ... One or more unquoted expressions separated by commas. #' Expressions matching values in the column defined by `by` determine #' deceased authors. Matching of values is case-insensitive. - #' @param by Variable used to specify whether an author is deceased or not. + #' @param .by Variable used to specify whether an author is deceased or not. #' By default, uses authors' id. + #' @param by `r lifecycle::badge("deprecated")` + #' + #' Please use the `.by` parameter instead. #' @return The class instance. - set_deceased = function(..., by) { - private$set_status("deceased", ..., by = by) + set_deceased = function(..., .by, by = deprecated()) { + private$set_status("deceased", ..., .by = .by, by = by) } ) ) @@ -139,7 +151,7 @@ StatusSetterPlumeQuarto <- R6Class( #' #' aut$set_corresponding_authors( #' everyone_but(jean), -#' by = "given_name" +#' .by = "given_name" #' ) #' aut$get_plume() |> dplyr::select(1:3, corresponding) #' @export diff --git a/README.Rmd b/README.Rmd index 9f4297e..337b743 100644 --- a/README.Rmd +++ b/README.Rmd @@ -92,7 +92,7 @@ Alternatively, you can generate author information as character strings using `P ```{r, message = FALSE} aut <- Plume$new(encyclopedists) -aut$set_corresponding_authors(diderot, by = "family_name") +aut$set_corresponding_authors(diderot, .by = "family_name") aut$get_author_list(format = "^a,^cn") |> enumerate(last = ",\n") diff --git a/README.md b/README.md index f69730b..be2d7bc 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ using `Plume`: ``` r aut <- Plume$new(encyclopedists) -aut$set_corresponding_authors(diderot, by = "family_name") +aut$set_corresponding_authors(diderot, .by = "family_name") aut$get_author_list(format = "^a,^cn") |> enumerate(last = ",\n") #> Denis Diderot^1,^\*†, Jean-Jacques Rousseau^2^, François-Marie Arouet^2^‡, diff --git a/man/StatusSetter.Rd b/man/StatusSetter.Rd index de23917..04c9c7f 100644 --- a/man/StatusSetter.Rd +++ b/man/StatusSetter.Rd @@ -40,7 +40,7 @@ Internal class that manages authors' status. \subsection{Method \code{set_corresponding_authors()}}{ Set corresponding authors. \subsection{Usage}{ -\if{html}{\out{
}}\preformatted{StatusSetter$set_corresponding_authors(..., by)}\if{html}{\out{
}} +\if{html}{\out{
}}\preformatted{StatusSetter$set_corresponding_authors(..., .by, by = deprecated())}\if{html}{\out{
}} } \subsection{Arguments}{ @@ -50,8 +50,12 @@ Set corresponding authors. Expressions matching values in the column defined by \code{by} determine corresponding authors. Matching of values is case-insensitive.} -\item{\code{by}}{Variable used to set corresponding authors. By default, uses +\item{\code{.by}}{Variable used to set corresponding authors. By default, uses authors' id.} + +\item{\code{by}}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} + +Please use the \code{.by} parameter instead.} } \if{html}{\out{}} } diff --git a/man/StatusSetterPlumeQuarto.Rd b/man/StatusSetterPlumeQuarto.Rd index 0973494..b1e6d2c 100644 --- a/man/StatusSetterPlumeQuarto.Rd +++ b/man/StatusSetterPlumeQuarto.Rd @@ -33,7 +33,7 @@ Internal class extending \code{StatusSetter} for \code{PlumeQuarto}. \subsection{Method \code{set_equal_contributor()}}{ Set equal contributors. \subsection{Usage}{ -\if{html}{\out{
}}\preformatted{StatusSetterPlumeQuarto$set_equal_contributor(..., by)}\if{html}{\out{
}} +\if{html}{\out{
}}\preformatted{StatusSetterPlumeQuarto$set_equal_contributor(..., .by, by = deprecated())}\if{html}{\out{
}} } \subsection{Arguments}{ @@ -43,8 +43,12 @@ Set equal contributors. Expressions matching values in the column defined by \code{by} determine equal contributors. Matching of values is case-insensitive.} -\item{\code{by}}{Variable used to specify which authors are equal contributors. +\item{\code{.by}}{Variable used to specify which authors are equal contributors. By default, uses authors' id.} + +\item{\code{by}}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} + +Please use the \code{.by} parameter instead.} } \if{html}{\out{}} } @@ -58,7 +62,7 @@ The class instance. \subsection{Method \code{set_deceased()}}{ Set deceased authors. \subsection{Usage}{ -\if{html}{\out{
}}\preformatted{StatusSetterPlumeQuarto$set_deceased(..., by)}\if{html}{\out{
}} +\if{html}{\out{
}}\preformatted{StatusSetterPlumeQuarto$set_deceased(..., .by, by = deprecated())}\if{html}{\out{
}} } \subsection{Arguments}{ @@ -68,8 +72,12 @@ Set deceased authors. Expressions matching values in the column defined by \code{by} determine deceased authors. Matching of values is case-insensitive.} -\item{\code{by}}{Variable used to specify whether an author is deceased or not. +\item{\code{.by}}{Variable used to specify whether an author is deceased or not. By default, uses authors' id.} + +\item{\code{by}}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} + +Please use the \code{.by} parameter instead.} } \if{html}{\out{}} } diff --git a/man/everyone.Rd b/man/everyone.Rd index a49ff36..bc9e8b6 100644 --- a/man/everyone.Rd +++ b/man/everyone.Rd @@ -30,7 +30,7 @@ aut$get_plume() |> dplyr::select(1:3, corresponding) aut$set_corresponding_authors( everyone_but(jean), - by = "given_name" + .by = "given_name" ) aut$get_plume() |> dplyr::select(1:3, corresponding) } diff --git a/tests/testthat/_snaps/set-status.md b/tests/testthat/_snaps/set-status.md index f7520a7..6f61c7c 100644 --- a/tests/testthat/_snaps/set-status.md +++ b/tests/testthat/_snaps/set-status.md @@ -1,59 +1,69 @@ +# the `by` parameter is deprecated + + Code + aut <- Plume$new(basic_df) + aut$set_corresponding_authors(zip, by = "given_name") + Condition + Warning: + The `by` argument of `set_corresponding_author()` is deprecated as of plume 0.2.0. + i Please use the `.by` argument instead. + # set_*() methods give meaningful error messages Code - (expect_error(aut$set_corresponding_authors(a, by = "foo"))) + (expect_error(aut$set_corresponding_authors(a, .by = "foo"))) Output Error: ! Column `foo` doesn't exist. Code - (expect_error(aut$set_corresponding_authors(a, by = ""))) + (expect_error(aut$set_corresponding_authors(a, .by = ""))) Output Error: - ! `by` must be a non-empty string. + ! `.by` must be a non-empty string. Code - (expect_error(aut$set_corresponding_authors(a, by = 1))) + (expect_error(aut$set_corresponding_authors(a, .by = 1))) Output Error: - ! `by` must be a character string. + ! `.by` must be a character string. Code - (expect_error(aut$set_equal_contributor(a, by = "foo"))) + (expect_error(aut$set_equal_contributor(a, .by = "foo"))) Output Error: ! Column `foo` doesn't exist. Code - (expect_error(aut$set_equal_contributor(a, by = ""))) + (expect_error(aut$set_equal_contributor(a, .by = ""))) Output Error: - ! `by` must be a non-empty string. + ! `.by` must be a non-empty string. Code - (expect_error(aut$set_equal_contributor(a, by = 1))) + (expect_error(aut$set_equal_contributor(a, .by = 1))) Output Error: - ! `by` must be a character string. + ! `.by` must be a character string. Code - (expect_error(aut$set_deceased(a, by = "foo"))) + (expect_error(aut$set_deceased(a, .by = "foo"))) Output Error: ! Column `foo` doesn't exist. Code - (expect_error(aut$set_deceased(a, by = ""))) + (expect_error(aut$set_deceased(a, .by = ""))) Output Error: - ! `by` must be a non-empty string. + ! `.by` must be a non-empty string. Code - (expect_error(aut$set_deceased(a, by = 1))) + (expect_error(aut$set_deceased(a, .by = 1))) Output Error: - ! `by` must be a character string. + ! `.by` must be a character string. # everyone*() selectors error if used in a wrong context diff --git a/tests/testthat/test-set-status.R b/tests/testthat/test-set-status.R index 3c84286..2660a3a 100644 --- a/tests/testthat/test-set-status.R +++ b/tests/testthat/test-set-status.R @@ -9,7 +9,7 @@ test_that("sets status to selected authors", { }, rep(TRUE, 3)) expect_equal({ - aut$set_corresponding_authors(everyone_but(ric), by = "given_name") + aut$set_corresponding_authors(everyone_but(ric), .by = "given_name") aut$get_plume()$corresponding }, c(TRUE, FALSE, TRUE)) @@ -24,25 +24,35 @@ test_that("sets status to selected authors", { }, c(TRUE, FALSE, TRUE)) expect_equal({ - aut$set_corresponding_authors(zip, by = "given_name") + aut$set_corresponding_authors(zip, .by = "given_name") aut$get_plume()$corresponding }, c(TRUE, FALSE, FALSE)) # set_equal_contributor expect_equal({ - aut$set_equal_contributor(zip, by = "given_name") + aut$set_equal_contributor(zip, .by = "given_name") aut$get_plume()$equal_contributor }, c(TRUE, FALSE, FALSE)) # set_deceased expect_equal({ - aut$set_deceased(zip, by = "given_name") + aut$set_deceased(zip, .by = "given_name") aut$get_plume()$deceased }, c(TRUE, FALSE, FALSE)) }) +# Deprecation ---- + +test_that("the `by` parameter is deprecated", { + expect_snapshot({ + aut <- Plume$new(basic_df) + aut$set_corresponding_authors(zip, by = "given_name") + }) + expect_equal(aut$get_plume()$corresponding, c(TRUE, FALSE, FALSE)) +}) + # Errors ---- test_that("set_*() methods give meaningful error messages", { @@ -50,31 +60,31 @@ test_that("set_*() methods give meaningful error messages", { expect_snapshot({ (expect_error( - aut$set_corresponding_authors(a, by = "foo") + aut$set_corresponding_authors(a, .by = "foo") )) (expect_error( - aut$set_corresponding_authors(a, by = "") + aut$set_corresponding_authors(a, .by = "") )) (expect_error( - aut$set_corresponding_authors(a, by = 1) + aut$set_corresponding_authors(a, .by = 1) )) (expect_error( - aut$set_equal_contributor(a, by = "foo") + aut$set_equal_contributor(a, .by = "foo") )) (expect_error( - aut$set_equal_contributor(a, by = "") + aut$set_equal_contributor(a, .by = "") )) (expect_error( - aut$set_equal_contributor(a, by = 1) + aut$set_equal_contributor(a, .by = 1) )) (expect_error( - aut$set_deceased(a, by = "foo") + aut$set_deceased(a, .by = "foo") )) (expect_error( - aut$set_deceased(a, by = "") + aut$set_deceased(a, .by = "") )) (expect_error( - aut$set_deceased(a, by = 1) + aut$set_deceased(a, .by = 1) )) }) }) diff --git a/vignettes/plume.Rmd b/vignettes/plume.Rmd index 024d67f..6014ecf 100644 --- a/vignettes/plume.Rmd +++ b/vignettes/plume.Rmd @@ -242,7 +242,7 @@ Note that these methods are case insensitive. ```{r} aut <- Plume$new(dplyr::select(encyclopedists, given_name, family_name)) -aut$set_corresponding_authors(dd, "j-jr", by = "initials") +aut$set_corresponding_authors(dd, "j-jr", .by = "initials") aut ``` @@ -256,7 +256,7 @@ aut Or its variant `everyone_but()` to assign `TRUE` to all but specific authors: ```{r} -aut$set_corresponding_authors(everyone_but(jean), by = "given_name") +aut$set_corresponding_authors(everyone_but(jean), .by = "given_name") aut ```