diff --git a/NEWS.md b/NEWS.md index ca608c6..7f17f01 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,29 +2,27 @@ ## 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). ## 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). +* 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 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( 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) } diff --git a/tests/testthat/test-check_package.R b/tests/testthat/test-check_package.R index 8e622db..896bfca 100644 --- a/tests/testthat/test-check_package.R +++ b/tests/testthat/test-check_package.R @@ -37,53 +37,68 @@ 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" ) }) +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 + 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)) })