From 6ba8ce1dd704b5fa5ecf16481f4009614b1e2dc5 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Sat, 11 Apr 2026 16:22:45 +0200 Subject: [PATCH 1/7] Name parameters --- R/deprecated.R | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/R/deprecated.R b/R/deprecated.R index 0606bc7..9e3e27b 100644 --- a/R/deprecated.R +++ b/R/deprecated.R @@ -11,9 +11,9 @@ #' @name deprecated get_schema <- function(package, resource_name) { lifecycle::deprecate_soft( - "1.3.0", - "get_schema()", - "schema()" + when = "1.3.0", + what = "get_schema()", + with = "schema()" ) schema(package, resource_name) } @@ -27,9 +27,9 @@ get_schema <- function(package, resource_name) { #' @name deprecated resources <- function(package) { lifecycle::deprecate_soft( - "1.3.0", - "resources()", - "resource_names()" + when = "1.3.0", + what = "resources()", + with = "resource_names()" ) resource_names(package) } From 1b4cbb2649ea8b64177453669d1940a3eb13c070 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Sat, 11 Apr 2026 16:23:01 +0200 Subject: [PATCH 2/7] Put pkgs in backticks --- NEWS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index ca608c6..0a22dda 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,7 @@ ## For users -* `read_resource()` now supports reading from remote zip files, thanks to support in {vroom} (1.3.0) (#291). +* `read_resource()` now supports reading from remote zip files, thanks to support in `{vroom}` (1.3.0) (#291). * `resources()` is soft-deprecated, please use `resource_names()` instead (#282). * `get_schema()` is soft-deprecated, please use `schema()` instead (#282). @@ -24,7 +24,7 @@ attr(r, "data_location") # Renamed! ``` -* frictionless now relies on R >= 4.1.0 (because of an indirect {vroom} dependency) (#291) and uses base pipes (`|>` rather than `%>%`) (#292). +* frictionless now relies on R >= 4.1.0 (because of an indirect `{vroom}` dependency) (#291) and uses base pipes (`|>` rather than `%>%`) (#292). # frictionless 1.2.1 From 42153f17283bcb256730666b7e7ab18447167b23 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Sat, 11 Apr 2026 16:23:15 +0200 Subject: [PATCH 3/7] Handle deprecated package$directory --- R/check_package.R | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/R/check_package.R b/R/check_package.R index da24f22..4602ebb 100644 --- a/R/check_package.R +++ b/R/check_package.R @@ -53,6 +53,18 @@ check_package <- function(package) { ) } + # Handle deprecated package$directory property + if (is.character(package$directory)) { + lifecycle::deprecate_warn( + when = "1.3.0", + what = I("`package$directory`"), + details = "This Data Package was created with an older version of + frictionless. Read or create it again to avoid this warning." + ) + attr(package, "directory") <- package$directory + package$directory <- NULL + } + # Check package has directory attribute (character) if (!is.character(attr(package, "directory"))) { cli::cli_abort( From eac552e05b92f42dad99fb1f620d05dfb09fd2e9 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Sat, 11 Apr 2026 16:23:23 +0200 Subject: [PATCH 4/7] Add test --- tests/testthat/test-check_package.R | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/testthat/test-check_package.R b/tests/testthat/test-check_package.R index 8e622db..db76dcd 100644 --- a/tests/testthat/test-check_package.R +++ b/tests/testthat/test-check_package.R @@ -70,6 +70,15 @@ test_that("check_package() returns error on missing or incorrect directory", { ) }) +test_that("check_package() returns deprecation warning for package$directory", { + p_directory_prop <- example_package() + p_directory_prop$directory <- attr(p_directory_prop, "directory") + attr(p_directory_prop, "directory") <- NULL + + lifecycle::expect_deprecated(check_package(p_directory_prop)) + expect_no_error(suppressWarnings(check_package(p_directory_prop))) +}) + test_that("check_package() returns error if resources have no name", { p <- example_package() p$resources[[2]]$name <- NULL From 8ca4b699040751ac5e97d4abf87b97dfd7920e87 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Sat, 11 Apr 2026 16:24:20 +0200 Subject: [PATCH 5/7] Don't rely on order of checks for tests Rather, create a valid package and remove the element to test --- tests/testthat/test-check_package.R | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/testthat/test-check_package.R b/tests/testthat/test-check_package.R index db76dcd..896bfca 100644 --- a/tests/testthat/test-check_package.R +++ b/tests/testthat/test-check_package.R @@ -37,35 +37,40 @@ test_that("check_package() returns error if package is not a list", { }) test_that("check_package() returns error on missing or incorrect resources", { + p_invalid <- create_package() + p_invalid$resources <- NULL expect_error( - check_package(list()), + check_package(p_invalid), class = "frictionless_error_package_invalid" ) + p_invalid$resources <- "not_a_list" expect_error( - check_package(list(resources = "not_a_list")), + check_package(p_invalid), regexp = "`package` is missing a resources property or it is not a list.", fixed = TRUE ) expect_error( - check_package(list(resources = "not_a_list")), + check_package(p_invalid), class = "frictionless_error_package_invalid" ) }) test_that("check_package() returns error on missing or incorrect directory", { + p_invalid <- create_package() + attr(p_invalid, "directory") <- NULL expect_error( - check_package(list(resources = list())), + check_package(p_invalid), class = "frictionless_error_package_invalid" ) expect_error( - check_package(list(resources = list())), + check_package(p_invalid), regexp = paste( "`package` is missing a directory attribute or it is not a character." ), fixed = TRUE ) expect_error( - check_package(list(resources = list(), directory = 5)), + check_package(p_invalid), class = "frictionless_error_package_invalid" ) }) @@ -80,19 +85,20 @@ test_that("check_package() returns deprecation warning for package$directory", { }) test_that("check_package() returns error if resources have no name", { - p <- example_package() - p$resources[[2]]$name <- NULL + p_invalid <- example_package() + p_invalid$resources[[2]]$name <- NULL expect_error( - check_package(p), + check_package(p_invalid), class = "frictionless_error_resources_without_name" ) expect_error( - check_package(p), + check_package(p_invalid), regexp = "All resources in `package` must have a name property.", fixed = TRUE ) # Expect no error on empty resources + p <- example_package() p$resources <- list() expect_no_error(check_package(p)) }) From 51ae0011f45779da574b4dc48333830e3e93d5d1 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Sat, 11 Apr 2026 16:37:50 +0200 Subject: [PATCH 6/7] Describe change --- NEWS.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0a22dda..7f17f01 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,20 +8,18 @@ ## Changes for developers -* Internal frictionless properties are now _attributes_, to better separate them from public Data Package properties (#289). If you use these internal properties, then update: +* Internal frictionless properties `package$directory` and `resource$read_from` are now _attributes_ `attr(package, "directory")` and `attr(resource, "data_location")`. This separates them better from public Data Package and Resource _properties_ (#289). Saved Data Package objects created with previous versions of frictionless will show a deprecation warning (#293). If you use these internal properties in your R package, then update them: ```R + # Before package$directory - r <- frictionless:::get_resource(package, "resource_name") # Internal function - r$read_from - ``` - - to: + resource <- frictionless:::get_resource(package, "resource_name") # Internal function + resource$read_from - ```R + # After attr(package, "directory") - r <- frictionless:::resource(package, "resource_name") # Renamed! - attr(r, "data_location") # Renamed! + resource <- frictionless:::resource(package, "resource_name") # Function renamed! + attr(resource, "data_location") # Attribute renamed! ``` * frictionless now relies on R >= 4.1.0 (because of an indirect `{vroom}` dependency) (#291) and uses base pipes (`|>` rather than `%>%`) (#292). From 94a1194ec63ac1b745696e1032efc25f35e0fa36 Mon Sep 17 00:00:00 2001 From: Peter Desmet Date: Wed, 13 May 2026 11:35:49 +0200 Subject: [PATCH 7/7] Update package$directory with create_package() Not with check_package()! --- NEWS.md | 6 +++--- R/check_package.R | 15 ++++++--------- R/create_package.R | 6 +++++- tests/testthat/test-check_package.R | 10 +++++----- tests/testthat/test-create_package.R | 10 ++++++++++ 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7f17f01..1614ad1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,7 +8,7 @@ ## Changes for developers -* Internal frictionless properties `package$directory` and `resource$read_from` are now _attributes_ `attr(package, "directory")` and `attr(resource, "data_location")`. This separates them better from public Data Package and Resource _properties_ (#289). Saved Data Package objects created with previous versions of frictionless will show a deprecation warning (#293). If you use these internal properties in your R package, then update them: +* Internal frictionless properties `package$directory` and `resource$read_from` are now _attributes_ `attr(package, "directory")` and `attr(resource, "data_location")`. This separates them better from public Data Package and Resource _properties_ (#289). Saved Data Package objects created with previous versions of frictionless will show a deprecation warning (#293) and can be updated with `create_package()`. If you use these internal properties in your R package, then change them: ```R # Before @@ -18,8 +18,8 @@ # After attr(package, "directory") - resource <- frictionless:::resource(package, "resource_name") # Function renamed! - attr(resource, "data_location") # Attribute renamed! + resource <- frictionless:::resource(package, "resource_name") # Function renamed + attr(resource, "data_location") # Attribute renamed ``` * frictionless now relies on R >= 4.1.0 (because of an indirect `{vroom}` dependency) (#291) and uses base pipes (`|>` rather than `%>%`) (#292). diff --git a/R/check_package.R b/R/check_package.R index 4602ebb..4391064 100644 --- a/R/check_package.R +++ b/R/check_package.R @@ -53,20 +53,17 @@ check_package <- function(package) { ) } - # Handle deprecated package$directory property + # Check package has directory attribute (or deprecated package$directory) if (is.character(package$directory)) { lifecycle::deprecate_warn( when = "1.3.0", what = I("`package$directory`"), - details = "This Data Package was created with an older version of - frictionless. Read or create it again to avoid this warning." + details = cli::format_inline( + "This Data Package object was created with an older version of + frictionless. Update it with {.fun create_package}." + ) ) - attr(package, "directory") <- package$directory - package$directory <- NULL - } - - # Check package has directory attribute (character) - if (!is.character(attr(package, "directory"))) { + } else if (!is.character(attr(package, "directory"))) { cli::cli_abort( c( general_message, diff --git a/R/create_package.R b/R/create_package.R index fb22de2..92d35ce 100644 --- a/R/create_package.R +++ b/R/create_package.R @@ -39,8 +39,12 @@ create_package <- function(descriptor = NULL) { # Add resources property (also creates descriptor if NULL) descriptor$resources <- descriptor$resources %||% list() - # Add directory attribute + # Add directory attribute (and remove deprecated package$directory) attr(descriptor, "directory") <- attr(descriptor, "directory") %||% "." + if (is.character(descriptor$directory)) { + attr(descriptor, "directory") <- descriptor$directory + descriptor$directory <- NULL + } # Add datapackage class if (!"datapackage" %in% class(descriptor)) { diff --git a/tests/testthat/test-check_package.R b/tests/testthat/test-check_package.R index 896bfca..9762299 100644 --- a/tests/testthat/test-check_package.R +++ b/tests/testthat/test-check_package.R @@ -76,12 +76,12 @@ test_that("check_package() returns error on missing or incorrect directory", { }) test_that("check_package() returns deprecation warning for package$directory", { - p_directory_prop <- example_package() - p_directory_prop$directory <- attr(p_directory_prop, "directory") - attr(p_directory_prop, "directory") <- NULL + p_directory <- example_package() + p_directory$directory <- attr(p_directory, "directory") + attr(p_directory, "directory") <- NULL - lifecycle::expect_deprecated(check_package(p_directory_prop)) - expect_no_error(suppressWarnings(check_package(p_directory_prop))) + lifecycle::expect_deprecated(check_package(p_directory)) + expect_no_error(suppressWarnings(check_package(p_directory))) }) test_that("check_package() returns error if resources have no name", { diff --git a/tests/testthat/test-create_package.R b/tests/testthat/test-create_package.R index d0005ab..b54c598 100644 --- a/tests/testthat/test-create_package.R +++ b/tests/testthat/test-create_package.R @@ -43,6 +43,16 @@ test_that("create_package() sets directory or leaves as is", { expect_identical(attr(existing, "directory"), "not_default") }) +test_that("create_package() updates deprecated package$directory", { + p_directory <- example_package() + p_directory$directory <- "not_default" + attr(p_directory, "directory") <- NULL + p_updated <- create_package(p_directory) + + expect_null(p_updated$directory) + expect_identical(attr(p_updated, "directory"), "not_default") +}) + test_that("create_package() adds class 'datapackage'", { new <- create_package() expect_s3_class(new, "datapackage")