Adjust class, typeof, colnames checks & messaging, gcd messaging

- S3 class vectors are ordered, so use `identical`
  - Improve class vector formatting
  - Tweak other `class` and `typeof` message text
- Improve duplicate colnames message
  - Improve vector interpolation formatting
- Fix typo in GCD error messaging

Author: brookslogan

NAMESPACE

@@ -98,6 +98,7 @@ importFrom(checkmate,test_subset)
 importFrom(checkmate,vname)
 importFrom(cli,cli_abort)
 importFrom(cli,cli_inform)
+importFrom(cli,cli_vec)
 importFrom(cli,cli_warn)
 importFrom(data.table,":=")
 importFrom(data.table,address)

R/archive.R

@@ -46,17 +46,17 @@ validate_version_bound <- function(version_bound, x, na_ok = FALSE,
       )
     }
   } else {
-    if (!test_set_equal(class(version_bound), class(x[["version"]]))) {
+    if (!identical(class(version_bound), class(x[["version"]]))) {
       cli_abort(
-        "{version_bound_arg} must have the same classes as x$version,
-        which is {class(x$version)}",
+        "{version_bound_arg} must have the same `class` vector as x$version,
+        which has a `class` of {paste(collapse = ' ', deparse(class(x$version)))}",
         class = "epiprocess__version_bound_mismatched_class"
       )
     }
     if (!identical(typeof(version_bound), typeof(x[["version"]]))) {
       cli_abort(
-        "{version_bound_arg} must have the same type as x$version,
-        which is {typeof(x$version)}",
+        "{version_bound_arg} must have the same `typeof` as x$version,
+        which has a `typeof` of {typeof(x$version)}",
         class = "epiprocess__version_bound_mismatched_typeof"
       )
     }

R/methods-epi_archive.R

@@ -68,14 +68,14 @@ epix_as_of <- function(x, max_version, min_time_value = -Inf, all_versions = FAL
   if (length(other_keys) == 0) other_keys <- NULL
 
   # Check a few things on max_version
-  if (!test_set_equal(class(max_version), class(x$DT$version))) {
+  if (!identical(class(max_version), class(x$DT$version))) {
     cli_abort(
-      "`max_version` must have the same classes as `epi_archive$DT$version`."
+      "`max_version` must have the same `class` vector as `epi_archive$DT$version`."
     )
   }
-  if (!test_set_equal(typeof(max_version), typeof(x$DT$version))) {
+  if (!identical(typeof(max_version), typeof(x$DT$version))) {
     cli_abort(
-      "`max_version` must have the same types as `epi_archive$DT$version`."
+      "`max_version` must have the same `typeof` as `epi_archive$DT$version`."
     )
   }
   assert_scalar(max_version, na.ok = FALSE)
@@ -859,11 +859,11 @@ epix_truncate_versions_after <- function(x, max_version) {
 #' @rdname epix_truncate_versions_after
 #' @export
 epix_truncate_versions_after.epi_archive <- function(x, max_version) {
-  if (!test_set_equal(class(max_version), class(x$DT$version))) {
-    cli_abort("`max_version` must have the same classes as `epi_archive$DT$version`.")
+  if (!identical(class(max_version), class(x$DT$version))) {
+    cli_abort("`max_version` must have the same `class` as `epi_archive$DT$version`.")
   }
-  if (!test_set_equal(typeof(max_version), typeof(x$DT$version))) {
-    cli_abort("`max_version` must have the same types as `epi_archive$DT$version`.")
+  if (!identical(typeof(max_version), typeof(x$DT$version))) {
+    cli_abort("`max_version` must have the same `typeof` as `epi_archive$DT$version`.")
   }
   assert_scalar(max_version, na.ok = FALSE)
   if (max_version > x$versions_end) {

R/methods-epi_df.R

@@ -123,6 +123,7 @@ decay_epi_df <- function(x) {
 #' @param template `epi_df` template to use to restore
 #' @return `epi_df` or degrade into `tbl_df`
 #' @importFrom dplyr dplyr_reconstruct
+#' @importFrom cli cli_vec
 #' @export
 #' @noRd
 dplyr_reconstruct.epi_df <- function(data, template) {
@@ -135,9 +136,11 @@ dplyr_reconstruct.epi_df <- function(data, template) {
   # Duplicate columns, cli_abort
   dup_col_names <- cn[duplicated(cn)]
   if (length(dup_col_names) != 0) {
-    cli_abort(paste0(
-      "Column name(s) {unique(dup_col_names)}",
-      "must not be duplicated."
+    cli_abort(c(
+      "Duplicate column names are not allowed",
+      "i" = "Duplicated column name{?s}:
+        {cli_vec(unique(dup_col_names),
+                 style = list('vec-sep2' = ', ', 'vec-last' = ', '))}"
     ))
   }
 

R/utils.R

@@ -609,7 +609,7 @@ gcd2num <- function(a, b, rrtol = 1e-6, pqlim = 1e6, irtol = 1e-6) {
     # `b_curr` is the candidate GCD / iterand; check first if it seems too small:
     if (abs(b_curr) <= iatol) {
       cli_abort(
-        "No GCD found; remaining potential Gads are all too small relative
+        "No GCD found; remaining potential GCDs are all too small relative
         to one/both of the original inputs; see `irtol` setting."
       )
     }

tests/testthat/test-archive-version-bounds.R

@@ -51,30 +51,30 @@ test_that("`validate_version_bound` validate and class checks together allow and
   my_version_bound1 <- `class<-`(24, "c1")
   expect_error(
     validate_version_bound(my_version_bound1, x_int, na_ok = FALSE),
-    regexp = "must have the same classes as"
+    regexp = "must have the same `class` vector as"
   )
   my_version_bound2 <- `class<-`(list(12), c("c2a", "c2b", "c2c"))
-  expect_error(validate_version_bound(my_version_bound2, x_list, na_ok = FALSE), regexp = "must have the same classes")
+  expect_error(validate_version_bound(my_version_bound2, x_list, na_ok = FALSE), regexp = "must have the same `class`")
   # Want no error matching date to date or datetime to datetime, but no interop due to tz issues:
   validate_version_bound(my_date, x_date, version_bound_arg = "vb")
   validate_version_bound(my_datetime, x_datetime, version_bound_arg = "vb")
   expect_error(
     validate_version_bound(my_datetime, x_date, na_ok = TRUE, version_bound_arg = "vb"),
-    regexp = "must have the same classes",
+    regexp = "must have the same `class`",
     class = "epiprocess__version_bound_mismatched_class"
   )
   expect_error(
     validate_version_bound(my_date, x_datetime, na_ok = TRUE, version_bound_arg = "vb"),
-    regexp = "must have the same classes",
+    regexp = "must have the same `class`",
     class = "epiprocess__version_bound_mismatched_class"
   )
   # Bad:
-  expect_error(validate_version_bound(3.5, x_int, TRUE, "vb"), regexp = "must have the same classes")
-  expect_error(validate_version_bound(.Machine$integer.max, x_dbl, TRUE, "vb"), regexp = "must have the same classes")
+  expect_error(validate_version_bound(3.5, x_int, TRUE, "vb"), regexp = "must have the same `class`")
+  expect_error(validate_version_bound(.Machine$integer.max, x_dbl, TRUE, "vb"), regexp = "must have the same `class`")
   expect_error(validate_version_bound(
     `class<-`(list(2), "clazz"),
     tibble::tibble(version = `class<-`(5L, "clazz")), TRUE, "vb"
-  ), regexp = "must have the same type", class = "epiprocess__version_bound_mismatched_typeof")
+  ), regexp = "must have the same `typeof`", class = "epiprocess__version_bound_mismatched_typeof")
   # Maybe questionable:
   expect_error(validate_version_bound(3, x_int, TRUE, "vb"))
   expect_error(validate_version_bound(3L, x_dbl, TRUE, "vb"))
@@ -99,7 +99,7 @@ test_that("archive version bounds args work as intended", {
       clobberable_versions_start = 1241,
       versions_end = measurement_date
     ),
-    regexp = "must have the same classes"
+    regexp = "must have the same `class`"
   )
   expect_error(
     as_epi_archive(update_tbl[integer(0L), ]),

tests/testthat/test-methods-epi_df.R

@@ -84,11 +84,11 @@ test_that("Subsetting drops & does not drop the epi_df class appropriately", {
 
 test_that("When duplicate cols in subset should abort", {
   expect_error(toy_epi_df[, c(2, 2:3, 4, 4, 4)],
-    "Column name(s) time_value, y must not be duplicated.",
+    "Duplicated column names: time_value, y",
     fixed = TRUE
   )
   expect_error(toy_epi_df[1:4, c(1, 2:4, 1)],
-    "Column name(s) geo_value must not be duplicated.",
+    "Duplicated column name: geo_value",
     fixed = TRUE
   )
 })