Updates from full package review

DESCRIPTION

@@ -2,8 +2,10 @@ Package: datatagr
 Title: Generic Data Labelling and Validating
 Version: 0.0.1
 Authors@R: c(
-    person("Chris", "Hartgerink", , "[email protected]", role = "cre",
-           comment = c(ORCID = "0000-0003-1050-6809"))
+    person("Chris", "Hartgerink", , "[email protected]", role = c("cre", "aut"),
+           comment = c(ORCID = "0000-0003-1050-6809")),
+    person("Hugo", "Gruson", , "[email protected]", role = "rev",
+           comment = c(ORCID = "0000-0002-4094-1476"))
   )
 Description: Provides tools to help label and validate data according to user-specified rules. The 'datatagr' class adds variable level attributes to 'data.frame' columns. Once labelled, these variables can be seamlessly used in downstream analyses, making data pipelines clearer, more robust, and more reliable.
 License: MIT + file LICENSE

R/datatagr-package.R

@@ -1,7 +1,7 @@
 #' Base Tools for Labelling and Validating Data
 #'
-#' The *datatagr* package provides tools to help label and validate data. The
-#' 'datatagr' class adds column level attributes to a 'data.frame'.
+#' The \pkg{datatagr} package provides tools to help label and validate data.
+#' The 'datatagr' class adds column level attributes to a 'data.frame'.
 #' Once labelled, variables can be seamlessly used in downstream analyses,
 #' making data pipelines more robust and reliable.
 #'

R/label_variables.R

@@ -39,6 +39,10 @@ label_variables <- function(x, labels) {
   for (name in names(labels)) {
     label_value <- unlist(labels[names(labels) == name])
 
+    # We use the `label` attribute to store the label
+    # We use `ifelse` to handle the case where the label is set to `NULL`
+    # This is because `as.character(NULL)` returns `character(0)`
+    # attr(x[[name]], "label") <- NULL does not have the desired result
     attr(x[[name]], "label") <- ifelse(is.null(label_value),
       "",
       as.character(label_value)

R/labels_df.R

@@ -25,10 +25,8 @@ labels_df <- function(x) {
   labels <- unlist(labels(x))
   out <- drop_datatagr(x)
 
-  # Find the intersection of names(out) and names(labels)
-  common_names <- intersect(names(out), names(labels))
   # Replace the names of out that are in intersection with corresponding labels
-  names(out)[match(common_names, names(out))] <- labels[common_names]
+  names(out)[match(names(labels), names(out))] <- labels[names(labels)]
 
   out
 }

R/lost_labels.R

@@ -16,11 +16,7 @@ lost_labels <- function(old, new, lost_action) {
   if (lost_action != "none" && length(lost_vars) > 0) {
     lost_labels <- lapply(lost_vars, function(label) old[[label]])
 
-    lost_msg <- paste(lost_vars,
-      lost_labels,
-      sep = " - ",
-      collapse = ", "
-    )
+    lost_msg <- vars_labels(lost_vars, lost_labels)
     msg <- paste(
       "The following labelled variables are lost:\n",
       lost_msg

R/print.datatagr.R

@@ -38,12 +38,13 @@ print.datatagr <- function(x, ...) {
   label_names <- names(label_values)
 
   # Construct the labels_txt string from the filtered pairs
-  labels_txt <- paste(label_names, label_values, sep = "-", collapse = ", ")
+  labels_txt <- vars_labels(label_names, label_values)
 
   if (labels_txt == "") {
-    labels_txt <- "[no labelled variables]"
+    cat("\n[no labelled variables]\n")
+  } else {
+    cat("\nlabelled variables:\n", labels_txt, "\n")
   }
-  cat("\nlabels:", labels_txt, "\n")
 
   invisible(x)
 }

R/restore_labels.R

@@ -38,11 +38,7 @@ restore_labels <- function(x, newLabels,
   if (lost_action != "none" && length(lost_vars) > 0) {
     lost_labels <- lapply(lost_vars, function(label) newLabels[[label]])
 
-    lost_msg <- paste(lost_vars,
-      lost_labels,
-      sep = " - ",
-      collapse = ", "
-    )
+    lost_msg <- vars_labels(lost_vars, lost_labels)
     msg <- paste(
       "The following labelled variables are lost:\n",
       lost_msg

R/validate_labels.R

@@ -37,7 +37,7 @@ validate_labels <- function(x) {
 
   if (is.null(unlist(x_labels))) stop("`x` has no labels")
 
-  # check that x is a list, and each label is a `character`
+  # check that x_labels is a list, and each label is a `character`
   checkmate::assert_list(x_labels, types = c("character", "null"))
 
   x

R/validate_types.R

@@ -37,8 +37,7 @@
 validate_types <- function(x, ...) {
   checkmate::assert_class(x, "datatagr")
   types <- rlang::list2(...)
-  checkmate::assert_list(types, min.len = 1)
-  checkmate::assert_list(types, types = "character")
+  checkmate::assert_list(types, min.len = 1, types = "character")
 
   vars_to_check <- intersect(names(x), names(types))
 

R/vars_labels.R

@@ -0,0 +1,11 @@
+#' Internal printing function for variables and labels
+#'
+#' @param vars a `character` vector of variable names
+#' @param labels a `character` vector of labels
+vars_labels <- function(vars, labels) {
+  paste(vars,
+    labels,
+    sep = " - ",
+    collapse = "\n "
+  )
+}

README.Rmd

@@ -76,8 +76,8 @@ To make it easier for us to evaluate your contribution, please run the following
 
 ```r
 styler::style_pkg()
-spelling::update_wordlist(pkg = ".", vignettes = TRUE)
 devtools::document()
+spelling::update_wordlist(pkg = ".", vignettes = TRUE)
 
 lintr::lint_package()
 
@@ -94,6 +94,7 @@ This will reduce the time it takes for us to review your contribution. Thank you
 
 This project is related to other existing projects in R or other languages, but also differs from them in the following aspects:
 
+- [labelled](https://github.com/larmarange/labelled/): A package for labelling data in R, but it is more focused on labelling variables than validating them.
 - [linelist](https://github.com/epiverse-trace/linelist): A package for managing and validating linelist data - the original inspiration for datatagr.
 
 ### Code of Conduct

README.md

@@ -81,8 +81,8 @@ is consistent with the rest of the package:
 
 ``` r
 styler::style_pkg()
-spelling::update_wordlist(pkg = ".", vignettes = TRUE)
 devtools::document()
+spelling::update_wordlist(pkg = ".", vignettes = TRUE)
 
 lintr::lint_package()
 
@@ -100,6 +100,9 @@ Thank you! 😊
 This project is related to other existing projects in R or other
 languages, but also differs from them in the following aspects:
 
+- [labelled](https://github.com/larmarange/labelled/): A package for
+  labelling data in R, but it is more focused on labelling variables
+  than validating them.
 - [linelist](https://github.com/epiverse-trace/linelist): A package for
   managing and validating linelist data - the original inspiration for
   datatagr.

inst/WORDLIST

@@ -6,7 +6,12 @@ ORCID
 RECON
 Unlabeled
 dplyr
+leverspeed
 lifecycle
 linelist
+messspeeds
 rlang
+tibble
 tidyselect
+tidyverse
+unclassing

tests/testthat/_snaps/compat-dplyr.md

@@ -1,7 +1,8 @@
 # Compatibility with dplyr::transmute()
 
     The following labelled variables are lost:
-     speed - Miles per hour, dist - Distance in miles
+     speed - Miles per hour
+     dist - Distance in miles
 
 # Compatibility with dplyr::mutate(.keep)
 

tests/testthat/_snaps/print.md

@@ -54,7 +54,9 @@
     49    24  120
     50    25   85
 
-    labels: speed-Miles per hour, dist-Distance in miles 
+    labelled variables:
+     speed - Miles per hour
+     dist - Distance in miles 
 
 ---
 
@@ -112,5 +114,5 @@
     49    24  120
     50    25   85
 
-    labels: [no labelled variables] 
+    [no labelled variables]
 

tests/testthat/test-square_bracket.R

@@ -14,7 +14,7 @@ test_that("tests for [ operator", {
   expect_error(x[, 1], msg)
 
   lost_labels_action("warning", quiet = TRUE)
-  msg <- "The following labelled variables are lost:\n speed - Miles per hour, dist - Distance in miles"
+  msg <- "The following labelled variables are lost:\n speed - Miles per hour\n dist - Distance in miles"
   expect_warning(x[, NULL], msg)
 
   # functionalities