From 913ea8cad82fbcc8523f6738c8e409af22d78c95 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 11:38:24 -0600 Subject: [PATCH 1/8] Move the dots in `pin_write()` --- R/pin-read-write.R | 6 +++++- man/pin_read.Rd | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/R/pin-read-write.R b/R/pin-read-write.R index a7a8c887..a1be276c 100644 --- a/R/pin-read-write.R +++ b/R/pin-read-write.R @@ -71,15 +71,19 @@ pin_read <- function(board, name, version = NULL, hash = NULL, ...) { #' @export pin_write <- function(board, x, name = NULL, + ..., type = NULL, title = NULL, description = NULL, metadata = NULL, versioned = NULL, tags = NULL, - ..., force_identical_write = FALSE) { check_board(board, "pin_write", "pin") + dots <- list2(...) + if (!is_empty(dots) && is.null(names(dots[1]))) { + cli::cli_abort('The {.arg type} argument must be named, like {.code type = "{dots[[1]]}"}.') + } if (is.null(name)) { name <- enexpr(x) diff --git a/man/pin_read.Rd b/man/pin_read.Rd index aa15768c..fdc80697 100644 --- a/man/pin_read.Rd +++ b/man/pin_read.Rd @@ -11,13 +11,13 @@ pin_write( board, x, name = NULL, + ..., type = NULL, title = NULL, description = NULL, metadata = NULL, versioned = NULL, tags = NULL, - ..., force_identical_write = FALSE ) } From a13533b20072a816ae0755e27e4c7627c776f25d Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 11:38:42 -0600 Subject: [PATCH 2/8] Update tests --- tests/testthat/_snaps/pin-read-write.md | 5 +++++ tests/testthat/test-pin-read-write.R | 3 ++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/testthat/_snaps/pin-read-write.md b/tests/testthat/_snaps/pin-read-write.md index fad76a59..2d3ea0df 100644 --- a/tests/testthat/_snaps/pin-read-write.md +++ b/tests/testthat/_snaps/pin-read-write.md @@ -19,6 +19,11 @@ Condition Error in `pin_write()`: ! `name` must be a string + Code + pin_write(board, mtcars, name = 1:10, "json") + Condition + Error in `pin_write()`: + ! The `type` argument must be named, like `type = "json"`. Code pin_write(board, mtcars, name = "mtcars", type = "froopy-loops") Condition diff --git a/tests/testthat/test-pin-read-write.R b/tests/testthat/test-pin-read-write.R index 593532d5..8fbdc311 100644 --- a/tests/testthat/test-pin-read-write.R +++ b/tests/testthat/test-pin-read-write.R @@ -41,6 +41,7 @@ test_that("useful errors on bad inputs", { expect_snapshot(error = TRUE, { pin_write(mtcars) pin_write(board, mtcars, name = 1:10) + pin_write(board, mtcars, name = 1:10, "json") pin_write(board, mtcars, name = "mtcars", type = "froopy-loops") pin_write(board, mtcars, name = "mtcars", metadata = 1) }) @@ -65,7 +66,7 @@ test_that("pin_write() noisily generates name and type", { test_that("user can supply metadata", { board <- board_temp() - pin_write(board, 1:10, "x", metadata = list(name = "Susan"), desc = "A vector") + pin_write(board, 1:10, "x", metadata = list(name = "Susan"), description = "A vector") meta <- pin_meta(board, "x") expect_equal(meta$user, list(name = "Susan")) expect_equal(meta$description, "A vector") From e0467e70632604b7a347cf0d341db864d769edcd Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 11:49:24 -0600 Subject: [PATCH 3/8] Better error message --- R/pin-read-write.R | 2 +- tests/testthat/_snaps/pin-read-write.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/pin-read-write.R b/R/pin-read-write.R index a1be276c..8590bb3e 100644 --- a/R/pin-read-write.R +++ b/R/pin-read-write.R @@ -82,7 +82,7 @@ pin_write <- function(board, x, check_board(board, "pin_write", "pin") dots <- list2(...) if (!is_empty(dots) && is.null(names(dots[1]))) { - cli::cli_abort('The {.arg type} argument must be named, like {.code type = "{dots[[1]]}"}.') + cli::cli_abort('Arguments after the dots `...` must be named, like {.code type = "{dots[[1]]}"}.') } if (is.null(name)) { diff --git a/tests/testthat/_snaps/pin-read-write.md b/tests/testthat/_snaps/pin-read-write.md index 2d3ea0df..9a83a965 100644 --- a/tests/testthat/_snaps/pin-read-write.md +++ b/tests/testthat/_snaps/pin-read-write.md @@ -23,7 +23,7 @@ pin_write(board, mtcars, name = 1:10, "json") Condition Error in `pin_write()`: - ! The `type` argument must be named, like `type = "json"`. + ! Arguments after the dots `...` must be named, like `type = "json"`. Code pin_write(board, mtcars, name = "mtcars", type = "froopy-loops") Condition From e6f8128d68e6a24001c90e78e16cd8b5209b9d26 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 12:08:13 -0600 Subject: [PATCH 4/8] Update NEWS --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index ba82882e..e1e46f0c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ # pins (development version) +* Changed the function signature of `pin_write()` so arguments like `type` and `title` must be passed by name and not position (#792). + # pins 1.2.2 * Fixed how dots are checked in `pin_write()` to make user-facing messages more From dd11c9b9c7472d29a5cee2c088af649092f90229 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 12:41:54 -0600 Subject: [PATCH 5/8] Better checking of names of dots --- R/pin-read-write.R | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/R/pin-read-write.R b/R/pin-read-write.R index 8590bb3e..56dfc438 100644 --- a/R/pin-read-write.R +++ b/R/pin-read-write.R @@ -80,9 +80,8 @@ pin_write <- function(board, x, tags = NULL, force_identical_write = FALSE) { check_board(board, "pin_write", "pin") - dots <- list2(...) - if (!is_empty(dots) && is.null(names(dots[1]))) { - cli::cli_abort('Arguments after the dots `...` must be named, like {.code type = "{dots[[1]]}"}.') + if (!missing(...) && (is.null(...names()) || ...names()[[1]] == "")) { + cli::cli_abort('Arguments after the dots `...` must be named, like {.code type = "json"}.') } if (is.null(name)) { From 7cd9f3141b8ca3be7731ff8861e91cc44d90abef Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 12:42:09 -0600 Subject: [PATCH 6/8] Fix test case --- tests/testthat/_snaps/pin-read-write.md | 2 +- tests/testthat/test-pin-read-write.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/_snaps/pin-read-write.md b/tests/testthat/_snaps/pin-read-write.md index 9a83a965..dc94ca7a 100644 --- a/tests/testthat/_snaps/pin-read-write.md +++ b/tests/testthat/_snaps/pin-read-write.md @@ -20,7 +20,7 @@ Error in `pin_write()`: ! `name` must be a string Code - pin_write(board, mtcars, name = 1:10, "json") + pin_write(board, mtcars, name = "mtcars", "json") Condition Error in `pin_write()`: ! Arguments after the dots `...` must be named, like `type = "json"`. diff --git a/tests/testthat/test-pin-read-write.R b/tests/testthat/test-pin-read-write.R index 8fbdc311..0ba84050 100644 --- a/tests/testthat/test-pin-read-write.R +++ b/tests/testthat/test-pin-read-write.R @@ -41,7 +41,7 @@ test_that("useful errors on bad inputs", { expect_snapshot(error = TRUE, { pin_write(mtcars) pin_write(board, mtcars, name = 1:10) - pin_write(board, mtcars, name = 1:10, "json") + pin_write(board, mtcars, name = "mtcars", "json") pin_write(board, mtcars, name = "mtcars", type = "froopy-loops") pin_write(board, mtcars, name = "mtcars", metadata = 1) }) From 3ef78675e669182917b9093b7850c4dde972f745 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 12:43:31 -0600 Subject: [PATCH 7/8] Let's call this a breaking change --- NEWS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index e1e46f0c..49b6e7de 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,6 @@ -# pins (development version) +# pins (development version to be released as 1.3.0) + +## Breaking changes * Changed the function signature of `pin_write()` so arguments like `type` and `title` must be passed by name and not position (#792). From f789629f9d0423c466fa431bdd523fcc28740748 Mon Sep 17 00:00:00 2001 From: Julia Silge Date: Fri, 29 Sep 2023 12:50:33 -0600 Subject: [PATCH 8/8] Don't have `...names()` available for all our R versions yet --- R/pin-read-write.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/R/pin-read-write.R b/R/pin-read-write.R index 56dfc438..a7405c13 100644 --- a/R/pin-read-write.R +++ b/R/pin-read-write.R @@ -80,7 +80,8 @@ pin_write <- function(board, x, tags = NULL, force_identical_write = FALSE) { check_board(board, "pin_write", "pin") - if (!missing(...) && (is.null(...names()) || ...names()[[1]] == "")) { + dots <- list2(...) + if (!missing(...) && (is.null(names(dots)) || names(dots)[[1]] == "")) { cli::cli_abort('Arguments after the dots `...` must be named, like {.code type = "json"}.') }