Merge pull request #289 from frictionlessdata/attributes

Use attributes

Author: peterdesmet

NEWS.md

@@ -1,8 +1,29 @@
 # frictionless (development version)
 
+## For users
+
+* `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).
-* `read_resource()` now supports reading from remote zip files, thanks to support in {vroom} (1.3.0) (#291).
+
+## 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:
+
+  ```R
+  package$directory
+  r <- frictionless:::get_resource(package, "resource_name") # Internal function
+  r$read_from
+  ```
+  
+  to:
+  
+  ```R
+  attr(package, "directory")
+  r <- frictionless:::resource(package, "resource_name") # Renamed!
+  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 1.2.1

R/check_package.R

@@ -44,28 +44,28 @@ check_package <- function(package) {
     )
   }
 
-  # Check package has directory (character)
-  if (!is.character(package$directory)) {
+  # Check all resources (if any) have a name
+  if (purrr::some(package$resources, ~ is.null(.x$name))) {
+    cli::cli_abort(
+      "All {.field resources} in {.arg package} must have a {.field name}
+       property.",
+      class = "frictionless_error_resources_without_name"
+    )
+  }
+
+  # Check package has directory attribute (character)
+  if (!is.character(attr(package, "directory"))) {
     cli::cli_abort(
       c(
         general_message,
-        "x" = "{.arg package} is missing a {.field directory} property or it is
+        "x" = "{.arg package} is missing a {.field directory} attribute or it is
                not a character.",
         "i" = tip_message
       ),
       class = "frictionless_error_package_invalid"
     )
   }
 
-  # Check all resources (if any) have a name
-  if (purrr::some(package$resources, ~ is.null(.x$name))) {
-    cli::cli_abort(
-      "All {.field resources} in {.arg package} must have a {.field name}
-       property.",
-      class = "frictionless_error_resources_without_name"
-    )
-  }
-
   # Warn if class is missing
   if (!"datapackage" %in% class(package)) {
     cli::cli_warn(

R/col_types.R

@@ -16,7 +16,7 @@ cols <- function(schema) {
 
   # Create col_types
   col_types <- purrr::map(fields, field_to_col)
-  # Assign names: list("name1" = <collector_character>, "name2" = ...)
+  # Add names: list("name1" = <collector_character>, "name2" = ...)
   names(col_types) <- field_names
 
   # Replicate structure of readr::col_spec
@@ -44,7 +44,7 @@ field_to_col <- function(field) {
   bare_number <- if (field$bareNumber %||% "" != FALSE) TRUE else FALSE
   format <- field$format %||% "default" # Undefined => default
 
-  # Assign types and formats
+  # Add types and formats
   col_type <- switch(
     type,
     "string" = col_string(enum),

R/create_package.R

@@ -3,12 +3,12 @@
 #' Initiates a Data Package object, either from scratch or from an existing
 #' list.
 #' This Data Package object is a list with the following characteristics:
-#' - A `datapackage` subclass.
 #' - All properties of the original `descriptor`.
 #' - A `resources` property, set to an empty list if undefined.
-#' - A `directory` property, set to `"."` for the current directory if
+#' - A `directory` attribute, set to `"."` for the current directory if
 #'   undefined.
 #'   It is used as the base path to access resources with [read_resource()].
+#' - A `datapackage` subclass.
 #'
 #' See `vignette("data-package")` to learn how this function implements the
 #' Data Package standard.
@@ -36,9 +36,11 @@ create_package <- function(descriptor = NULL) {
     )
   }
 
-  # Add properties
+  # Add resources property (also creates descriptor if NULL)
   descriptor$resources <- descriptor$resources %||% list()
-  descriptor$directory <- descriptor$directory %||% "." # Current directory
+
+  # Add directory attribute
+  attr(descriptor, "directory") <- attr(descriptor, "directory") %||% "."
 
   # Add datapackage class
   if (!"datapackage" %in% class(descriptor)) {

R/read_from_path.R

@@ -33,7 +33,11 @@ read_from_path <- function(package, resource_name, col_select) {
   col_types <- cols(schema)
 
   # Get dialect (can be NULL)
-  dialect <- read_descriptor(resource$dialect, package$directory, safe = TRUE)
+  dialect <- read_descriptor(
+    resource$dialect,
+    attr(package, "directory"),
+    safe = TRUE
+  )
   escape_backslash <- if (dialect$escapeChar %||% "not set" == "\\") {
     TRUE
   } else {

R/read_package.R

@@ -45,7 +45,7 @@ read_package <- function(file = "datapackage.json") {
   }
 
   # Add directory
-  descriptor$directory <- dirname(file) # Also works for URLs
+  attr(descriptor, "directory") <- dirname(file) # Also works for URLs
 
   # Create package
   create_package(descriptor)

R/read_resource.R

@@ -50,19 +50,20 @@ read_resource <- function(package, resource_name, col_select = NULL) {
   resource <- resource(package, resource_name)
 
   # Read data directly
-  if (resource$read_from == "df") {
+  data_location <- attr(resource, "data_location")
+  if (data_location == "df") {
     df <- dplyr::as_tibble(resource$data)
 
   # Read data from data
-  } else if (resource$read_from == "data") {
+  } else if (data_location == "data") {
     df <- do.call(
       function(...) rbind.data.frame(..., stringsAsFactors = FALSE),
       resource$data
     )
     df <- dplyr::as_tibble(df)
 
   # Read data from path(s)
-  } else if (resource$read_from == "path" || resource$read_from == "url") {
+  } else if (data_location == "path" || data_location == "url") {
     df <- read_from_path(package, resource_name, col_select)
   }
   return(df)

R/resource.R

@@ -4,8 +4,8 @@
 #' described `resources`.
 #'
 #' @inheritParams read_resource
-#' @return List describing a Data Resource, with new property `read_from` to
-#'   indicate how data should be read.
+#' @return List describing a Data Resource, with new attribute `data_location`
+#'   to indicate how the data are attached.
 #'   If present, `path` will be updated to contain the full path(s).
 #' @family accessor functions
 #' @noRd
@@ -47,25 +47,26 @@ resource <- function(package, resource_name) {
     )
   }
 
-  # Assign read_from property (based on path, then df, then data)
+  # Add data_location attribute (based on path, then df, then data)
   if (length(resource$path) != 0) {
     if (all(is_url(resource$path))) {
-      resource$read_from <- "url"
+      data_location <- "url"
     } else {
-      resource$read_from <- "path"
+      data_location <- "path"
     }
     # Expand paths to full paths, check if file exists and check path safety,
     # unless those paths were willingly added by user in add_resource()
     if (attr(resource, "path") %||% "" != "added") {
       resource$path <- purrr::map_chr(
-        resource$path, ~ check_path(.x, package$directory, safe = TRUE)
+        resource$path, ~ check_path(.x, attr(package, "directory"), safe = TRUE)
       )
     }
   } else if (is.data.frame(resource$data)) {
-    resource$read_from <- "df"
+    data_location <- "df"
   } else if (!is.null(resource$data)) {
-    resource$read_from <- "data"
+    data_location <- "data"
   }
+  attr(resource, "data_location") <- data_location
 
   return(resource)
 }

R/schema.R

@@ -39,7 +39,11 @@ schema <- function(package, resource_name) {
       class = "frictionless_error_resource_without_schema"
     )
   }
-  schema <- read_descriptor(resource$schema, package$directory, safe = TRUE)
+  schema <- read_descriptor(
+    resource$schema,
+    attr(package, "directory"),
+    safe = TRUE
+  )
 
   # Check schema
   check_schema(schema)

R/write_package.R

@@ -73,7 +73,7 @@ write_package <- function(package, directory, compress = FALSE) {
   return_package <- package # Needs directory to remain valid
 
   # Write datapackage.json
-  package$directory <- NULL
+  attr(package, "directory") <- NULL
   package_json <- jsonlite::toJSON(
     package,
     pretty = TRUE,