Deprecate by in favour of .by (#41)

In `set_*()` methods for consistency purposes

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).

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

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")
 

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^‡,

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/rlang_error>
       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/rlang_error>
       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/rlang_error>
       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/rlang_error>
       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/rlang_error>
       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/rlang_error>
       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/rlang_error>
       Error:
       ! Column `foo` doesn't exist.
     Code
-      (expect_error(aut$set_deceased(a, by = "")))
+      (expect_error(aut$set_deceased(a, .by = "")))
     Output
       <error/rlang_error>
       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/rlang_error>
       Error:
-      ! `by` must be a character string.
+      ! `.by` must be a character string.
 
 # everyone*() selectors error if used in a wrong context
 

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,57 +24,67 @@ 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", {
   aut <- PlumeQuarto$new(basic_df, tempfile_())
 
   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)
     ))
   })
 })

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
 ```