Show deprecation warning for Data Packages created with previous versions of frictionless

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
 

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(

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

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