From 89e927482e9a4bd0bc95fb824b802689622fffbd Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Wed, 10 Dec 2025 13:27:25 -0600
Subject: [PATCH 01/12] #109 add verbose argument

---
 R/build.R                   | 480 +++++++++++++++++++-----------------
 R/utils.R                   |  29 ++-
 man/drop_unspec_vars.Rd     |  14 +-
 tests/testthat/test-build.R | 216 ++++++++++------
 4 files changed, 423 insertions(+), 316 deletions(-)

diff --git a/R/build.R b/R/build.R
index e55da5a..a773a78 100644
--- a/R/build.R
+++ b/R/build.R
@@ -56,110 +56,114 @@
 #' build_from_derived(spec, ds_list, predecessor_only = FALSE)
 build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
                                predecessor_only = TRUE, keep = FALSE) {
-   if (is_present(dataset_name)) {
-      lifecycle::deprecate_warn(
-         when = "0.2.0",
-         what = "build_from_derived(dataset_name)",
-         details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
+  if (is_present(dataset_name)) {
+    lifecycle::deprecate_warn(
+      when = "0.2.0",
+      what = "build_from_derived(dataset_name)",
+      details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
       Please use {.fn metacore::select_dataset} to subset the {.obj metacore} object to obtain
       metadata for a single dataset.")))
-      )
-      metacore <- make_lone_dataset(metacore, dataset_name)
-   }
-   verify_DatasetMeta(metacore)
-
-   # Deprecate KEEP = TRUE
-   keep <- match.arg(as.character(keep), c("TRUE", "FALSE", "ALL", "PREREQUISITE"))
-   if (keep == "TRUE"){
-      cli_warn(paste0("Setting 'keep' = TRUE has been superseded",
+    )
+    metacore <- make_lone_dataset(metacore, dataset_name)
+  }
+  verify_DatasetMeta(metacore)
+
+  # Deprecate KEEP = TRUE
+  keep <- match.arg(as.character(keep), c("TRUE", "FALSE", "ALL", "PREREQUISITE"))
+  if (keep == "TRUE") {
+    cli_warn(paste0(
+      "Setting 'keep' = TRUE has been superseded",
       ", and will be unavailable in future releases. Please consider setting ",
-      "'keep' equal to 'ALL' or 'PREREQUISITE'."))
-   }
-
-   derirvations <- metacore$derivations %>%
-      mutate(derivation = trimws(derivation))
-
-   if (predecessor_only) {
-      limited_dev_ids <- metacore$value_spec %>%
-         filter(str_detect(str_to_lower(origin), "predecessor")) %>%
-         pull(derivation_id)
-
-      derirvations <- derirvations %>%
-         filter(derivation_id %in% limited_dev_ids)
-      if (nrow(derirvations) == 0) {
-         stop("No predecessor variables found please check your metacore object")
-      }
-   }
-
-   vars_to_pull_through <- derirvations %>%
-      filter(str_detect(derivation, "^\\w*\\.[a-zA-Z0-9]*$"))
-
-   # To lower so it is flexible about how people name their ds list
-   vars_w_ds <- vars_to_pull_through %>%
-      mutate(ds = str_extract(derivation, "^\\w*(?=\\.)") %>%
-                str_to_lower())
-   ds_names <- vars_w_ds %>%
-      pull(ds) %>%
-      unique()
-   if(is.null(names(ds_list))){
-      names(ds_list) <- deparse(substitute(ds_list)) %>%
-         str_remove("list\\s?\\(") %>%
-         str_remove("\\)s?$") %>%
-         str_split(",\\s?") %>%
-         unlist()
-   }
-   names(ds_list) <- names(ds_list) %>%
-      str_to_lower()
-   if (!all(ds_names %in% names(ds_list))) {
-      unknown <- keep(names(ds_list), ~!.%in% ds_names)
-      if(length(unknown) > 0){
-         warning(paste0("The following dataset(s) have no predecessors and will be ignored:\n"),
-                 paste0(unknown, collapse = ", "),
-                 call. = FALSE)
-      }
-      ds_using <- discard(names(ds_list), ~. %in% unknown) %>%
-         str_to_upper() %>%
-         paste0(collapse = ", ")
-
-      message(paste0(
-         "Not all datasets provided. Only variables from ",
-         ds_using,
-         " will be gathered."
-      ))
-
-      # Filter out any variable that come from datasets that aren't present
-      vars_w_ds <- vars_w_ds %>%
-         filter(ds %in% names(ds_list))
-
-   }
-
-   ds_keys <- metacore$ds_vars %>%
-      filter(!is.na(key_seq)) %>%
-      pull(variable)
-
-   joining_vals_to_add <- ds_list %>%
-      map(function(x){
-         names(x) %>%
-            keep(~ . %in% ds_keys)
-         })
-
-   join_by = joining_vals_to_add %>%
-      reduce(intersect)
-   additional_vals <- tibble(ds = names(ds_list),
-          variable = joining_vals_to_add) %>%
-      unnest(variable) %>%
-      mutate(col_name = variable)
-
-   vars_w_ds %>%
-      mutate(col_name = str_extract(derivation, "(?<=\\.).*")) %>%
-      inner_join(metacore$value_spec, ., by = "derivation_id") %>%
-      select(variable, ds, col_name) %>%
-      bind_rows(additional_vals) %>%
-      group_by(ds) %>%
-      group_split() %>%
-      map(get_variables, ds_list, keep, derirvations) %>%
-      prepare_join(join_by, names(ds_list)) %>%
-      reduce(full_join, by = join_by)
+      "'keep' equal to 'ALL' or 'PREREQUISITE'."
+    ))
+  }
+
+  derirvations <- metacore$derivations %>%
+    mutate(derivation = trimws(derivation))
+
+  if (predecessor_only) {
+    limited_dev_ids <- metacore$value_spec %>%
+      filter(str_detect(str_to_lower(origin), "predecessor")) %>%
+      pull(derivation_id)
+
+    derirvations <- derirvations %>%
+      filter(derivation_id %in% limited_dev_ids)
+    if (nrow(derirvations) == 0) {
+      stop("No predecessor variables found please check your metacore object")
+    }
+  }
+
+  vars_to_pull_through <- derirvations %>%
+    filter(str_detect(derivation, "^\\w*\\.[a-zA-Z0-9]*$"))
+
+  # To lower so it is flexible about how people name their ds list
+  vars_w_ds <- vars_to_pull_through %>%
+    mutate(ds = str_extract(derivation, "^\\w*(?=\\.)") %>%
+      str_to_lower())
+  ds_names <- vars_w_ds %>%
+    pull(ds) %>%
+    unique()
+  if (is.null(names(ds_list))) {
+    names(ds_list) <- deparse(substitute(ds_list)) %>%
+      str_remove("list\\s?\\(") %>%
+      str_remove("\\)s?$") %>%
+      str_split(",\\s?") %>%
+      unlist()
+  }
+  names(ds_list) <- names(ds_list) %>%
+    str_to_lower()
+  if (!all(ds_names %in% names(ds_list))) {
+    unknown <- keep(names(ds_list), ~ !. %in% ds_names)
+    if (length(unknown) > 0) {
+      warning(paste0("The following dataset(s) have no predecessors and will be ignored:\n"),
+        paste0(unknown, collapse = ", "),
+        call. = FALSE
+      )
+    }
+    ds_using <- discard(names(ds_list), ~ . %in% unknown) %>%
+      str_to_upper() %>%
+      paste0(collapse = ", ")
+
+    message(paste0(
+      "Not all datasets provided. Only variables from ",
+      ds_using,
+      " will be gathered."
+    ))
+
+    # Filter out any variable that come from datasets that aren't present
+    vars_w_ds <- vars_w_ds %>%
+      filter(ds %in% names(ds_list))
+  }
+
+  ds_keys <- metacore$ds_vars %>%
+    filter(!is.na(key_seq)) %>%
+    pull(variable)
+
+  joining_vals_to_add <- ds_list %>%
+    map(function(x) {
+      names(x) %>%
+        keep(~ . %in% ds_keys)
+    })
+
+  join_by <- joining_vals_to_add %>%
+    reduce(intersect)
+  additional_vals <- tibble(
+    ds = names(ds_list),
+    variable = joining_vals_to_add
+  ) %>%
+    unnest(variable) %>%
+    mutate(col_name = variable)
+
+  vars_w_ds %>%
+    mutate(col_name = str_extract(derivation, "(?<=\\.).*")) %>%
+    inner_join(metacore$value_spec, ., by = "derivation_id") %>%
+    select(variable, ds, col_name) %>%
+    bind_rows(additional_vals) %>%
+    group_by(ds) %>%
+    group_split() %>%
+    map(get_variables, ds_list, keep, derirvations) %>%
+    prepare_join(join_by, names(ds_list)) %>%
+    reduce(full_join, by = join_by)
 }
 
 #' Internal functions to get variables from a dataset list
@@ -174,46 +178,46 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
 #' @return datasets
 #' @noRd
 get_variables <- function(x, ds_list, keep, derivations) {
-   ds_name <- unique(x$ds)
-   data <- ds_list[[ds_name]]
-   rename_vec <- set_names(x$col_name, x$variable)
-   if (keep == "TRUE") {
-      # Don't drop predecessor columns
-      out <- data %>%
-         select(x$col_name) %>%
-         mutate(across(all_of(rename_vec)))
-   } else if (keep == "FALSE") {
-      # Drop predecessor columns
-      out <- data %>%
-         select(x$col_name) %>%
-         mutate(across(all_of(rename_vec))) %>%
-         select(x$variable)
-   } else if (keep == "ALL") {
-      # Keep all cols from original datasets
-      out <- data %>%
-         mutate(across(all_of(rename_vec)))
-   } else if (keep == "PREREQUISITE") {
-      # Keep all columns required for future derivations
-      # Find all "XX.XXXXX"
-      future_derivations <- derivations %>%
-         select(derivation) %>%
-         filter(!str_detect(derivation,"^[A-Z0-9a-z]+\\.[A-Z0-9a-z]+$"))
-
-      prereq_vector <- str_match_all(future_derivations$derivation, "([A-Z0-9a-z]+)\\.([A-Z0-9a-z]+)")
-
-      # Bind into matrix + remove dups
-      prereq_matrix <- do.call(rbind,prereq_vector) %>%
-         unique()
+  ds_name <- unique(x$ds)
+  data <- ds_list[[ds_name]]
+  rename_vec <- set_names(x$col_name, x$variable)
+  if (keep == "TRUE") {
+    # Don't drop predecessor columns
+    out <- data %>%
+      select(x$col_name) %>%
+      mutate(across(all_of(rename_vec)))
+  } else if (keep == "FALSE") {
+    # Drop predecessor columns
+    out <- data %>%
+      select(x$col_name) %>%
+      mutate(across(all_of(rename_vec))) %>%
+      select(x$variable)
+  } else if (keep == "ALL") {
+    # Keep all cols from original datasets
+    out <- data %>%
+      mutate(across(all_of(rename_vec)))
+  } else if (keep == "PREREQUISITE") {
+    # Keep all columns required for future derivations
+    # Find all "XX.XXXXX"
+    future_derivations <- derivations %>%
+      select(derivation) %>%
+      filter(!str_detect(derivation, "^[A-Z0-9a-z]+\\.[A-Z0-9a-z]+$"))
+
+    prereq_vector <- str_match_all(future_derivations$derivation, "([A-Z0-9a-z]+)\\.([A-Z0-9a-z]+)")
+
+    # Bind into matrix + remove dups
+    prereq_matrix <- do.call(rbind, prereq_vector) %>%
+      unique()
 
-      # Subset to those present in current dataset
-      prereq_cols <- subset(prereq_matrix, tolower(prereq_matrix[,2]) == tolower(ds_name))[,3]
+    # Subset to those present in current dataset
+    prereq_cols <- subset(prereq_matrix, tolower(prereq_matrix[, 2]) == tolower(ds_name))[, 3]
 
-      out <- data %>%
-         select(c(x$col_name, all_of(prereq_cols))) %>%
-         mutate(across(all_of(rename_vec))) %>%
-         select(c(x$variable, all_of(prereq_cols)))
-   }
-   out
+    out <- data %>%
+      select(c(x$col_name, all_of(prereq_cols))) %>%
+      mutate(across(all_of(rename_vec))) %>%
+      select(c(x$variable, all_of(prereq_cols)))
+  }
+  out
 }
 
 #' Internal function to remove duplicated non-key variables prior to join
@@ -232,29 +236,29 @@ get_variables <- function(x, ds_list, keep, derivations) {
 #' @return datasets
 #' @noRd
 prepare_join <- function(x, keys, ds_names) {
-   out <- list(x[[1]])
+  out <- list(x[[1]])
 
-   if (length(x) > 1){
-      for (i in 2:length(x)){
-         # Drop non-key cols present in each previous dataset in order
-         drop_cols <- c()
+  if (length(x) > 1) {
+    for (i in 2:length(x)) {
+      # Drop non-key cols present in each previous dataset in order
+      drop_cols <- c()
 
-         for (j in 1:(i-1)){
-            conflicting_cols <- keep(names(x[[j]]), function(col) !(col %in% keys)) %>%
-               intersect(colnames(x[[i]]))
-            drop_cols <- c(drop_cols, conflicting_cols)
+      for (j in 1:(i - 1)) {
+        conflicting_cols <- keep(names(x[[j]]), function(col) !(col %in% keys)) %>%
+          intersect(colnames(x[[i]]))
+        drop_cols <- c(drop_cols, conflicting_cols)
 
-            if(length(conflicting_cols) > 0){
-               cli_inform(c("i" = "Dropping column(s) from {ds_names[[i]]} due to \\
+        if (length(conflicting_cols) > 0) {
+          cli_inform(c("i" = "Dropping column(s) from {ds_names[[i]]} due to \\
                             conflict with {ds_names[[j]]}: {conflicting_cols}."))
-            }
-         }
-
-         out[[i]] <- x[[i]] %>%
-            select(-any_of(drop_cols))
+        }
       }
-   }
-   out
+
+      out[[i]] <- x[[i]] %>%
+        select(-any_of(drop_cols))
+    }
+  }
+  out
 }
 
 #' Drop Unspecified Variables
@@ -269,6 +273,12 @@ prepare_join <- function(x, keys, ds_names) {
 #' Note: Deprecated in version 0.2.0. The `dataset_name` argument will be removed
 #' in a future release. Please use `metacore::select_dataset` to subset the
 #' `metacore` object to obtain metadata for a single dataset.
+#' @param verbose Character string controlling message verbosity. One of:
+#'   \describe{
+#'     \item{`"message"`}{Show both warnings and messages (default)}
+#'     \item{`"warn"`}{Show warnings but suppress messages}
+#'     \item{`"silent"`}{Suppress all warnings and messages}
+#'   }
 #'
 #' @return Dataset with only specified columns
 #' @export
@@ -283,37 +293,43 @@ prepare_join <- function(x, keys, ds_names) {
 #'   select(USUBJID, SITEID) %>%
 #'   mutate(foo = "Hello")
 #' drop_unspec_vars(data, spec)
-drop_unspec_vars <- function(dataset, metacore, dataset_name = deprecated()) {
-   if (is_present(dataset_name)) {
-      lifecycle::deprecate_warn(
-         when = "0.2.0",
-         what = "drop_unspec_vars(dataset_name)",
-         details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
+drop_unspec_vars <- function(dataset, metacore, dataset_name = deprecated(),
+                             verbose = c("message", "warn", "silent")) {
+  if (is_present(dataset_name)) {
+    lifecycle::deprecate_warn(
+      when = "0.2.0",
+      what = "drop_unspec_vars(dataset_name)",
+      details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
       Please use {.fn metacore::select_dataset} to subset the {.obj metacore} object to obtain
       metadata for a single dataset.")))
-      )
-      metacore <- make_lone_dataset(metacore, dataset_name)
-   }
-
-   verify_DatasetMeta(metacore)
-   var_list <- metacore$ds_vars %>%
-      filter(is.na(supp_flag) | !(supp_flag)) %>%
-      pull(variable)
-   to_drop <- names(dataset) %>%
-      discard(~ . %in% var_list)
-   if (length(to_drop) > 0) {
-      out <- dataset %>%
-         select(-all_of(to_drop))
-      message(paste0("The following variable(s) were dropped:\n  ",
-              paste0(to_drop, collapse = "\n  ")))
-   } else {
-      out <- dataset
-   }
-   out
+    )
+    metacore <- make_lone_dataset(metacore, dataset_name)
+  }
+
+  verbose <- validate_verbose(verbose)
+
+  verify_DatasetMeta(metacore)
+  var_list <- metacore$ds_vars %>%
+    filter(is.na(supp_flag) | !(supp_flag)) %>%
+    pull(variable)
+  to_drop <- names(dataset) %>%
+    discard(~ . %in% var_list)
+  if (length(to_drop) > 0) {
+    out <- dataset %>%
+      select(-all_of(to_drop))
+    if (should_message(verbose)) {
+      message(paste0(
+        "The following variable(s) were dropped:\n  ",
+        paste0(to_drop, collapse = "\n  ")
+      ))
+    }
+  } else {
+    out <- dataset
+  }
+  out
 }
 
 
-
 #' Add Missing Variables
 #'
 #' This function adds in missing columns according to the type set in the
@@ -340,58 +356,60 @@ drop_unspec_vars <- function(dataset, metacore, dataset_name = deprecated()) {
 #' load(metacore_example("pilot_ADaM.rda"))
 #' spec <- metacore %>% select_dataset("ADSL")
 #' data <- read_xpt(metatools_example("adsl.xpt")) %>%
-#'    select(-TRTSDT, -TRT01P, -TRT01PN)
+#'   select(-TRTSDT, -TRT01P, -TRT01PN)
 #' add_variables(data, spec)
-add_variables <- function(dataset, metacore, dataset_name = deprecated()){
-   if (is_present(dataset_name)) {
-      lifecycle::deprecate_warn(
-         when = "0.2.0",
-         what = "add_variables(dataset_name)",
-         details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
+add_variables <- function(dataset, metacore, dataset_name = deprecated()) {
+  if (is_present(dataset_name)) {
+    lifecycle::deprecate_warn(
+      when = "0.2.0",
+      what = "add_variables(dataset_name)",
+      details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
       Please use {.fn metacore::select_dataset} to subset the {.obj metacore} object to obtain
       metadata for a single dataset.")))
+    )
+    metacore <- make_lone_dataset(metacore, dataset_name)
+  }
+
+  verify_DatasetMeta(metacore)
+  var_list <- metacore$ds_vars %>%
+    filter(is.na(supp_flag) | !(supp_flag)) %>%
+    pull(variable)
+
+  to_add <- var_list %>%
+    discard(~ . %in% names(dataset))
+  if (length(to_add) > 0) {
+    n <- nrow(dataset)
+    typing <- metacore$var_spec %>%
+      filter(variable %in% to_add) %>%
+      mutate(
+        type_fmt = str_to_lower(type),
+        out_type =
+          case_when(
+            str_detect(str_to_lower(format), "date") ~ "date",
+            type_fmt == "integer" ~ "integer",
+            type_fmt == "numeric" ~ "double",
+            type_fmt == "text" ~ "character",
+            type_fmt == "character" ~ "character",
+            type_fmt == "boolean" ~ "logical",
+            type_fmt == "logical" ~ "logical",
+            TRUE ~ "unknown"
+          )
       )
-      metacore <- make_lone_dataset(metacore, dataset_name)
-   }
-
-   verify_DatasetMeta(metacore)
-   var_list <- metacore$ds_vars %>%
-      filter(is.na(supp_flag) | !(supp_flag)) %>%
-      pull(variable)
 
-   to_add <- var_list %>%
-      discard(~ . %in% names(dataset))
-   if(length(to_add) > 0){
-      n <- nrow(dataset)
-      typing <- metacore$var_spec %>%
-         filter(variable %in% to_add) %>%
-         mutate(type_fmt = str_to_lower(type),
-                out_type =
-                   case_when(
-                      str_detect(str_to_lower(format), "date") ~ "date",
-                      type_fmt == "integer" ~ "integer",
-                      type_fmt == "numeric" ~ "double",
-                      type_fmt == "text" ~ "character",
-                      type_fmt == "character" ~ "character",
-                      type_fmt == "boolean" ~"logical",
-                      type_fmt == "logical" ~"logical",
-                      TRUE ~ "unknown"
-                   ))
-
-      new_cols <- map(typing$out_type, function(typ){
-         out <- switch(typ,
-                       "character" = rep(NA_character_, n),
-                       "integer" = rep(NA_integer_, n),
-                       "double" = rep(NA_real_, n),
-                       "date" = as.Date(rep(NA_integer_, n)),
-                       "logical" = rep(NA, n),
-                       "unknown" = rep(NA, n)
-         )
-      })
-      names(new_cols) <- typing$variable
-      new_cols <- as_tibble(new_cols)
+    new_cols <- map(typing$out_type, function(typ) {
+      out <- switch(typ,
+        "character" = rep(NA_character_, n),
+        "integer" = rep(NA_integer_, n),
+        "double" = rep(NA_real_, n),
+        "date" = as.Date(rep(NA_integer_, n)),
+        "logical" = rep(NA, n),
+        "unknown" = rep(NA, n)
+      )
+    })
+    names(new_cols) <- typing$variable
+    new_cols <- as_tibble(new_cols)
 
-      dataset <- bind_cols(dataset, new_cols)
-   }
-   dataset
+    dataset <- bind_cols(dataset, new_cols)
+  }
+  dataset
 }
diff --git a/R/utils.R b/R/utils.R
index cf251ed..b7bc5c8 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -32,10 +32,10 @@ metatools_example <- function(file = NULL) {
 #' @return metacore object
 #' @noRd
 make_lone_dataset <- function(metacore, dataset_name) {
-   lifecycle::deprecate_soft(
-      what = "make_lone_dataset()",
-      when = "0.2.0"
-   )
+  lifecycle::deprecate_soft(
+    what = "make_lone_dataset()",
+    when = "0.2.0"
+  )
   if (!(nrow(metacore$ds_spec) == 1 | !is.null(dataset_name))) {
     stop("Requires either a subsetted metacore object or a dataset name", call. = FALSE)
   }
@@ -44,3 +44,24 @@ make_lone_dataset <- function(metacore, dataset_name) {
   }
   metacore
 }
+
+#' Check if messages should be displayed
+#' @param verbose Verbosity level
+#' @noRd
+should_message <- function(verbose) {
+  verbose == "message"
+}
+
+#' Check if warnings should be displayed
+#' @param verbose Verbosity level
+#' @noRd
+should_warn <- function(verbose) {
+  verbose %in% c("message", "warn")
+}
+
+#' Validate verbose parameter
+#' @param verbose Verbosity level to validate
+#' @noRd
+validate_verbose <- function(verbose) {
+  match.arg(verbose, choices = c("message", "warn", "silent"))
+}
diff --git a/man/drop_unspec_vars.Rd b/man/drop_unspec_vars.Rd
index 5eb0c1d..dff1653 100644
--- a/man/drop_unspec_vars.Rd
+++ b/man/drop_unspec_vars.Rd
@@ -4,7 +4,12 @@
 \alias{drop_unspec_vars}
 \title{Drop Unspecified Variables}
 \usage{
-drop_unspec_vars(dataset, metacore, dataset_name = deprecated())
+drop_unspec_vars(
+  dataset,
+  metacore,
+  dataset_name = deprecated(),
+  verbose = c("message", "warn", "silent")
+)
 }
 \arguments{
 \item{dataset}{Dataset to change}
@@ -18,6 +23,13 @@ been subsetted.\cr
 Note: Deprecated in version 0.2.0. The \code{dataset_name} argument will be removed
 in a future release. Please use \code{metacore::select_dataset} to subset the
 \code{metacore} object to obtain metadata for a single dataset.}
+
+\item{verbose}{Character string controlling message verbosity. One of:
+\describe{
+\item{\code{"message"}}{Show both warnings and messages (default)}
+\item{\code{"warn"}}{Show warnings but suppress messages}
+\item{\code{"silent"}}{Suppress all warnings and messages}
+}}
 }
 \value{
 Dataset with only specified columns
diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R
index fc89c0a..9e6061d 100644
--- a/tests/testthat/test-build.R
+++ b/tests/testthat/test-build.R
@@ -1,11 +1,11 @@
 # Suppress cli output during testing
-options(cli.default_handler = function(...) { })
+options(cli.default_handler = function(...) {})
 
 load(metacore::metacore_example("pilot_ADaM.rda"))
 spec <- metacore %>% select_dataset("ADSL", quiet = TRUE)
 test_that("drop_unspec_vars", {
   data <- haven::read_xpt(metatools_example("adsl.xpt")) %>%
-    mutate(AGEGR2 = 'DUMMY', AGEGR2N = 99, foo = "Hello", foo2 = "world")
+    mutate(AGEGR2 = "DUMMY", AGEGR2N = 99, foo = "Hello", foo2 = "world")
 
   man_vars <- metacore$ds_vars %>%
     filter(dataset == "ADSL") %>%
@@ -15,8 +15,8 @@ test_that("drop_unspec_vars", {
   drop_unspec_vars(data, spec) %>%
     expect_equal(man_dat)
   expect_message(drop_unspec_vars(data, spec),
-                 label = "The following variable(s) were dropped:\n  foo\n  foo2")
-
+    label = "The following variable(s) were dropped:\n  foo\n  foo2"
+  )
 })
 
 
@@ -50,138 +50,194 @@ test_that("build_from_derived", {
     sort()
 
   expect_warning(
-     build_from_derived(spec, ds_list,
-       predecessor_only = FALSE,
-       keep = TRUE
-     ) %>%
-       names() %>%
-       sort() %>%
-       expect_equal(man_vars),
-     label = paste0("Setting 'keep' = TRUE has been superseded, and will be",
-        " unavailable in future releases. Please consider setting",
-        " 'keep' equal to 'ALL' or 'PREREQUISITE'.")
+    build_from_derived(spec, ds_list,
+      predecessor_only = FALSE,
+      keep = TRUE
+    ) %>%
+      names() %>%
+      sort() %>%
+      expect_equal(man_vars),
+    label = paste0(
+      "Setting 'keep' = TRUE has been superseded, and will be",
+      " unavailable in future releases. Please consider setting",
+      " 'keep' equal to 'ALL' or 'PREREQUISITE'."
+    )
   )
 
   # Pulling through from more than one dataset
   spec2 <- metacore %>% select_dataset("ADAE", quiet = TRUE)
   adae_auto <- build_from_derived(spec2,
-     ds_list = list("AE" = safetyData::sdtm_ae,
-                 "ADSL" = safetyData::adam_adsl),
-     predecessor_only = FALSE,
-     keep = FALSE
+    ds_list = list(
+      "AE" = safetyData::sdtm_ae,
+      "ADSL" = safetyData::adam_adsl
+    ),
+    predecessor_only = FALSE,
+    keep = FALSE
   )
   ae_part_vars <- spec2$derivations %>%
-     filter(str_detect(derivation,"AE\\.[[:alnum:]]*$")) %>%
-     pull(derivation) %>%
-     str_remove("^AE\\.") %>%
-     c("STUDYID", "USUBJID", .)
+    filter(str_detect(derivation, "AE\\.[[:alnum:]]*$")) %>%
+    pull(derivation) %>%
+    str_remove("^AE\\.") %>%
+    c("STUDYID", "USUBJID", .)
 
   ae_part <- select(safetyData::sdtm_ae, all_of(ae_part_vars))
 
   adsl_part_vars <- spec2$derivations %>%
-     filter(str_detect(derivation,"ADSL\\.[[:alnum:]]*$")) %>%
-     pull(derivation) %>%
-     str_remove("^ADSL\\.")
+    filter(str_detect(derivation, "ADSL\\.[[:alnum:]]*$")) %>%
+    pull(derivation) %>%
+    str_remove("^ADSL\\.")
   adsl_part <-
-     select(safetyData::adam_adsl, all_of(adsl_part_vars)) |>
-     rename(TRTA = TRT01A, TRTAN = TRT01AN)
+    select(safetyData::adam_adsl, all_of(adsl_part_vars)) |>
+    rename(TRTA = TRT01A, TRTAN = TRT01AN)
 
   adae_man <- full_join(adsl_part, ae_part, by = c("STUDYID", "USUBJID"), multiple = "all") %>%
-     select(all_of(names(adae_auto)), everything())
-  expect_equal(adae_auto,adae_man )
+    select(all_of(names(adae_auto)), everything())
+  expect_equal(adae_auto, adae_man)
 
 
   # Pulling through from one dataset when spec has more than one
   adae_auto_adsl_only <- build_from_derived(spec2,
-                                  ds_list = list("ADSL" = safetyData::adam_adsl),
-                                  predecessor_only = FALSE,
-                                  keep = FALSE
+    ds_list = list("ADSL" = safetyData::adam_adsl),
+    predecessor_only = FALSE,
+    keep = FALSE
   ) |>
-     order_cols(spec2)
+    order_cols(spec2)
   adsl_man <- order_cols(adsl_part, spec2)
   expect_equal(adae_auto_adsl_only, adsl_man)
 
-  adsl = safetyData::adam_adsl
-  ae = safetyData::sdtm_ae
+  adsl <- safetyData::adam_adsl
+  ae <- safetyData::sdtm_ae
   adae_auto_unnamed <- build_from_derived(spec2,
-                                  ds_list = list(ae, adsl),
-                                  predecessor_only = FALSE,
-                                  keep = FALSE
+    ds_list = list(ae, adsl),
+    predecessor_only = FALSE,
+    keep = FALSE
   )
-  expect_equal(adae_auto,adae_man)
+  expect_equal(adae_auto, adae_man)
 
   expect_warning(build_from_derived(spec2,
-                                    ds_list = list(safetyData::sdtm_ae, adsl),
-                                    predecessor_only = FALSE,
-                                    keep = FALSE
+    ds_list = list(safetyData::sdtm_ae, adsl),
+    predecessor_only = FALSE,
+    keep = FALSE
   ))
 
   # Pulling through all columns from original dataset
   adae_full <- build_from_derived(spec2,
-                                  ds_list = list("AE" = safetyData::sdtm_ae,
-                                                 "ADSL" = safetyData::adam_adsl),
-                                  predecessor_only = FALSE,
-                                  keep = "ALL"
+    ds_list = list(
+      "AE" = safetyData::sdtm_ae,
+      "ADSL" = safetyData::adam_adsl
+    ),
+    predecessor_only = FALSE,
+    keep = "ALL"
   )
 
   full_adsl_part <- safetyData::adam_adsl %>%
-     mutate(TRTA = TRT01A, TRTAN = TRT01AN)
+    mutate(TRTA = TRT01A, TRTAN = TRT01AN)
 
   adae_all_man <- full_join(full_adsl_part, safetyData::sdtm_ae, by = c("STUDYID", "USUBJID"), multiple = "all")
 
-  expect_equal(adae_full,adae_all_man)
+  expect_equal(adae_full, adae_all_man)
 
   # Pulling through columns required for future derivations
   spec3 <- metacore %>% select_dataset("ADAE", quiet = TRUE)
 
   adae_prereq <- build_from_derived(spec3,
-                                  ds_list = list("AE" = safetyData::sdtm_ae,
-                                                 "ADSL" = safetyData::adam_adsl),
-                                  predecessor_only = FALSE,
-                                  keep = "PREREQUISITE"
+    ds_list = list(
+      "AE" = safetyData::sdtm_ae,
+      "ADSL" = safetyData::adam_adsl
+    ),
+    predecessor_only = FALSE,
+    keep = "PREREQUISITE"
   )
 
   adae_auto <- build_from_derived(spec3,
-                                    ds_list = list("AE" = safetyData::sdtm_ae,
-                                                   "ADSL" = safetyData::adam_adsl),
-                                    predecessor_only = FALSE,
-                                    keep = "PREREQUISITE"
+    ds_list = list(
+      "AE" = safetyData::sdtm_ae,
+      "ADSL" = safetyData::adam_adsl
+    ),
+    predecessor_only = FALSE,
+    keep = "PREREQUISITE"
   )
 
 
   adae_all <- build_from_derived(spec3,
-                                 ds_list = list("AE" = safetyData::sdtm_ae,
-                                                "ADSL" = safetyData::adam_adsl),
-                                 predecessor_only = FALSE,
-                                 keep = "ALL"
+    ds_list = list(
+      "AE" = safetyData::sdtm_ae,
+      "ADSL" = safetyData::adam_adsl
+    ),
+    predecessor_only = FALSE,
+    keep = "ALL"
   )
 
   adae_prereq_man <- adae_all %>%
-     select(c(names(adae_auto)))
+    select(c(names(adae_auto)))
 
   expect_equal(adae_prereq, adae_prereq_man)
+})
 
+
+test_that("add_variables", {
+  load(metacore::metacore_example("pilot_ADaM.rda"))
+  spec <- metacore %>% select_dataset("ADSL", quiet = TRUE)
+  data <- haven::read_xpt(metatools_example("adsl.xpt")) %>%
+    mutate(AGEGR2 = "DUMMY", AGEGR2N = 99)
+  data_mis <- data %>%
+    select(-TRTSDT, -TRT01P, -TRT01PN)
+  # Check data when there is missing
+  fx_miss <- add_variables(data_mis, spec) %>%
+    select(TRTSDT, TRT01P, TRT01PN)
+  man_miss <- data %>%
+    mutate(
+      TRTSDT = as.Date(NA_integer_),
+      TRT01P = NA_character_,
+      TRT01PN = NA_integer_
+    ) %>%
+    select(TRTSDT, TRT01P, TRT01PN)
+  expect_equal(fx_miss, man_miss)
+  # Check data when there isn't any missing
+  expect_equal(
+    add_variables(data, spec),
+    data
+  )
 })
 
+test_that("drop_unspec_vars verbose parameter", {
+  data <- haven::read_xpt(metatools_example("adsl.xpt")) %>%
+    mutate(foo = "Hello", foo2 = "world")
 
+  man_vars <- metacore$ds_vars %>%
+    filter(dataset == "ADSL") %>%
+    pull(variable)
+  man_dat <- data %>%
+    select(all_of(man_vars))
 
-test_that("add_variables", {
-   load(metacore::metacore_example("pilot_ADaM.rda"))
-   spec <- metacore %>% select_dataset("ADSL", quiet = TRUE)
-   data <- haven::read_xpt(metatools_example("adsl.xpt")) %>%
-      mutate(AGEGR2 = "DUMMY", AGEGR2N = 99)
-   data_mis <- data %>%
-      select(-TRTSDT, -TRT01P, -TRT01PN)
-   #Check data when there is missing
-   fx_miss <- add_variables(data_mis, spec) %>%
-      select(TRTSDT, TRT01P, TRT01PN)
-   man_miss <- data %>%
-      mutate(TRTSDT = as.Date(NA_integer_),
-             TRT01P = NA_character_,
-             TRT01PN = NA_integer_) %>%
-      select(TRTSDT, TRT01P, TRT01PN)
-   expect_equal(fx_miss, man_miss)
-   #Check data when there isn't any missing
-   expect_equal(add_variables(data, spec),
-                data)
+  # Test verbose = "message" (default behavior)
+  expect_message(
+    drop_unspec_vars(data, spec, verbose = "message"),
+    "The following variable\\(s\\) were dropped:"
+  )
+
+  # Test verbose = "warn" (suppress messages)
+  expect_silent(
+    drop_unspec_vars(data, spec, verbose = "warn")
+  )
+
+  # Test verbose = "silent" (suppress all output)
+  expect_silent(
+    drop_unspec_vars(data, spec, verbose = "silent")
+  )
+
+  # Verify all verbose levels return same result
+  result_message <- drop_unspec_vars(data, spec, verbose = "message")
+  result_warn <- drop_unspec_vars(data, spec, verbose = "warn")
+  result_silent <- drop_unspec_vars(data, spec, verbose = "silent")
+
+  expect_equal(result_message, man_dat)
+  expect_equal(result_warn, man_dat)
+  expect_equal(result_silent, man_dat)
+
+  # Test invalid verbose value
+  expect_error(
+    drop_unspec_vars(data, spec, verbose = "invalid"),
+    "'arg' should be one of"
+  )
 })

From 2716c32dbfd050edb558286663993cec4a3d3050 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Wed, 10 Dec 2025 15:57:59 -0600
Subject: [PATCH 02/12] #109 add verbose to build_from_derived()

---
 R/build.R                   | 36 ++++++++++++++++++++++++------------
 man/build_from_derived.Rd   | 10 +++++++++-
 tests/testthat/test-build.R | 30 ++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/R/build.R b/R/build.R
index a773a78..37e1aab 100644
--- a/R/build.R
+++ b/R/build.R
@@ -41,7 +41,12 @@
 #'                            VS.VSTESTCD == 'Heart Rate'" contains both
 #'                            VS.VSTESTCD and VS.VSSTRESN as prerequisites, and
 #'                            these columns will be kept through to the ADaM.
-#'
+#' @param verbose Character string controlling message verbosity. One of:
+#'   \describe{
+#'     \item{`"message"`}{Show both warnings and messages (default)}
+#'     \item{`"warn"`}{Show warnings but suppress messages}
+#'     \item{`"silent"`}{Suppress all warnings and messages}
+#'   }
 #'
 #' @return dataset
 #' @export
@@ -55,7 +60,8 @@
 #' ds_list <- list(DM = read_xpt(metatools_example("dm.xpt")))
 #' build_from_derived(spec, ds_list, predecessor_only = FALSE)
 build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
-                               predecessor_only = TRUE, keep = FALSE) {
+                               predecessor_only = TRUE, keep = FALSE,
+                               verbose = c("message", "warn", "silent")) {
   if (is_present(dataset_name)) {
     lifecycle::deprecate_warn(
       when = "0.2.0",
@@ -67,6 +73,8 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
     metacore <- make_lone_dataset(metacore, dataset_name)
   }
   verify_DatasetMeta(metacore)
+  
+  verbose <- validate_verbose(verbose)
 
   # Deprecate KEEP = TRUE
   keep <- match.arg(as.character(keep), c("TRUE", "FALSE", "ALL", "PREREQUISITE"))
@@ -114,7 +122,7 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
     str_to_lower()
   if (!all(ds_names %in% names(ds_list))) {
     unknown <- keep(names(ds_list), ~ !. %in% ds_names)
-    if (length(unknown) > 0) {
+    if (length(unknown) > 0 && should_warn(verbose)) {
       warning(paste0("The following dataset(s) have no predecessors and will be ignored:\n"),
         paste0(unknown, collapse = ", "),
         call. = FALSE
@@ -124,11 +132,13 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
       str_to_upper() %>%
       paste0(collapse = ", ")
 
-    message(paste0(
-      "Not all datasets provided. Only variables from ",
-      ds_using,
-      " will be gathered."
-    ))
+    if (should_message(verbose)) {
+      message(paste0(
+        "Not all datasets provided. Only variables from ",
+        ds_using,
+        " will be gathered."
+      ))
+    }
 
     # Filter out any variable that come from datasets that aren't present
     vars_w_ds <- vars_w_ds %>%
@@ -162,7 +172,7 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
     group_by(ds) %>%
     group_split() %>%
     map(get_variables, ds_list, keep, derirvations) %>%
-    prepare_join(join_by, names(ds_list)) %>%
+    prepare_join(join_by, names(ds_list), verbose) %>%
     reduce(full_join, by = join_by)
 }
 
@@ -232,10 +242,12 @@ get_variables <- function(x, ds_list, keep, derivations) {
 #'
 #' @param x List of datasets with all columns added
 #' @param keys List of key values to join on
+#' @param ds_names Names of datasets
+#' @param verbose Verbosity level
 #'
 #' @return datasets
 #' @noRd
-prepare_join <- function(x, keys, ds_names) {
+prepare_join <- function(x, keys, ds_names, verbose = "message") {
   out <- list(x[[1]])
 
   if (length(x) > 1) {
@@ -248,8 +260,8 @@ prepare_join <- function(x, keys, ds_names) {
           intersect(colnames(x[[i]]))
         drop_cols <- c(drop_cols, conflicting_cols)
 
-        if (length(conflicting_cols) > 0) {
-          cli_inform(c("i" = "Dropping column(s) from {ds_names[[i]]} due to \\
+        if (length(conflicting_cols) > 0 && should_message(verbose)) {
+          cli_inform(c("i" = "Dropping column(s) from {ds_names[[i]]} due to \
                             conflict with {ds_names[[j]]}: {conflicting_cols}."))
         }
       }
diff --git a/man/build_from_derived.Rd b/man/build_from_derived.Rd
index 404840d..d75e421 100644
--- a/man/build_from_derived.Rd
+++ b/man/build_from_derived.Rd
@@ -9,7 +9,8 @@ build_from_derived(
   ds_list,
   dataset_name = deprecated(),
   predecessor_only = TRUE,
-  keep = FALSE
+  keep = FALSE,
+  verbose = c("message", "warn", "silent")
 )
 }
 \arguments{
@@ -54,6 +55,13 @@ VS.VSTESTCD == 'Heart Rate'" contains both
 VS.VSTESTCD and VS.VSSTRESN as prerequisites, and
 these columns will be kept through to the ADaM.
 }}
+
+\item{verbose}{Character string controlling message verbosity. One of:
+\describe{
+\item{\code{"message"}}{Show both warnings and messages (default)}
+\item{\code{"warn"}}{Show warnings but suppress messages}
+\item{\code{"silent"}}{Suppress all warnings and messages}
+}}
 }
 \value{
 dataset
diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R
index 9e6061d..2d49ec8 100644
--- a/tests/testthat/test-build.R
+++ b/tests/testthat/test-build.R
@@ -241,3 +241,33 @@ test_that("drop_unspec_vars verbose parameter", {
     "'arg' should be one of"
   )
 })
+
+test_that("build_from_derived verbose controls prepare_join messages", {
+  load(metacore::metacore_example("pilot_ADaM.rda"))
+  spec2 <- metacore %>% select_dataset("ADAE", quiet = TRUE)
+  
+  # Use safetyData datasets and add a conflicting non-key column
+  # STUDYID is a key column, so add a different column that will conflict
+  ae <- safetyData::sdtm_ae %>%
+    mutate(TESTCOL = "AE_VALUE")  # Add a non-key column
+  
+  adsl <- safetyData::adam_adsl %>%
+    mutate(TESTCOL = "ADSL_VALUE")  # Same column with different value
+  
+  ds_list <- list(AE = ae, ADSL = adsl)
+  
+  # Test that conflicting column messages are shown with verbose = "message"
+  expect_message(
+    build_from_derived(spec2, ds_list, predecessor_only = FALSE, verbose = "message", keep = "ALL"),
+    "Dropping column\\(s\\) from"
+  )
+  
+  # Test that messages are suppressed with verbose = "warn" and "silent"
+  expect_silent(
+    build_from_derived(spec2, ds_list, predecessor_only = FALSE, verbose = "warn", keep = "ALL")
+  )
+  
+  expect_silent(
+    build_from_derived(spec2, ds_list, predecessor_only = FALSE, verbose = "silent", keep = "ALL")
+  )
+})
\ No newline at end of file

From 6f78f29d3a595ca4fa6ee4494f1f576e351366ad Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Wed, 10 Dec 2025 16:40:23 -0600
Subject: [PATCH 03/12] #109 add verbose to build_qnam

---
 R/supp.R                   | 15 ++++++-
 man/build_qnam.Rd          | 17 +++++++-
 tests/testthat/test-supp.R | 80 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 109 insertions(+), 3 deletions(-)

diff --git a/R/supp.R b/R/supp.R
index ad24de0..eb4da6d 100644
--- a/R/supp.R
+++ b/R/supp.R
@@ -6,12 +6,21 @@
 #' @param idvar IDVAR variable name (provided as a string)
 #' @param qeval QEVAL value to be populated for this QNAM
 #' @param qorig QORIG value to be populated for this QNAM
+#' @param verbose Character string controlling message verbosity. One of:
+#'   \describe{
+#'     \item{`"message"`}{Show both warnings and messages (default)}
+#'     \item{`"warn"`}{Show warnings but suppress messages}
+#'     \item{`"silent"`}{Suppress all warnings and messages}
+#'   }
 #'
 #' @return Observations structured in SUPP format
 #' @export
 #'
 #'
-build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig) {
+build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig, 
+                       verbose = c("message", "warn", "silent")) {
+   verbose <- validate_verbose(verbose)
+   
    # Need QNAM as a variable
    qval <- as.symbol(qnam)
 
@@ -54,7 +63,9 @@ build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig) {
    blank_test <- out %>%
       pull(QVAL)
    if(any(blank_test == "")){
-      message(paste0("Empty QVAL rows removed for QNAM = ", unique(out$QNAM)))
+      if (should_message(verbose)) {
+         message(paste0("Empty QVAL rows removed for QNAM = ", unique(out$QNAM)))
+      }
       out <- out %>%
          filter(QVAL != "")
    }
diff --git a/man/build_qnam.Rd b/man/build_qnam.Rd
index 80afb31..e820ec6 100644
--- a/man/build_qnam.Rd
+++ b/man/build_qnam.Rd
@@ -4,7 +4,15 @@
 \alias{build_qnam}
 \title{Build the observations for a single QNAM}
 \usage{
-build_qnam(dataset, qnam, qlabel, idvar, qeval, qorig)
+build_qnam(
+  dataset,
+  qnam,
+  qlabel,
+  idvar,
+  qeval,
+  qorig,
+  verbose = c("message", "warn", "silent")
+)
 }
 \arguments{
 \item{dataset}{Input dataset}
@@ -18,6 +26,13 @@ build_qnam(dataset, qnam, qlabel, idvar, qeval, qorig)
 \item{qeval}{QEVAL value to be populated for this QNAM}
 
 \item{qorig}{QORIG value to be populated for this QNAM}
+
+\item{verbose}{Character string controlling message verbosity. One of:
+\describe{
+\item{\code{"message"}}{Show both warnings and messages (default)}
+\item{\code{"warn"}}{Show warnings but suppress messages}
+\item{\code{"silent"}}{Suppress all warnings and messages}
+}}
 }
 \value{
 Observations structured in SUPP format
diff --git a/tests/testthat/test-supp.R b/tests/testthat/test-supp.R
index eeb3a41..3d21732 100644
--- a/tests/testthat/test-supp.R
+++ b/tests/testthat/test-supp.R
@@ -272,3 +272,83 @@ test_that("combine_supp() does not create an IDVARVAL column (#78)", {
   noidvarval <- combine_supp(simple_ae, simple_suppae)
   expect_false("IDVARVAL" %in% names(noidvarval))
 })
+
+test_that("build_qnam verbose parameter", {
+  # Create simple test data with a column that will be used as QNAM
+  ae <- safetyData::sdtm_ae %>%
+    head(10) %>%
+    mutate(TESTVAR = c("", "", "Y", "Y", "N", "", "Y", "N", "Y", ""))  # Some empty strings
+  
+  # Test verbose = "message" (default) - should show message about empty QVAL
+  expect_message(
+    build_qnam(
+      dataset = ae,
+      qnam = "TESTVAR",
+      qlabel = "Test Variable",
+      idvar = "AESEQ",
+      qeval = "INVESTIGATOR",
+      qorig = "CRF",
+      verbose = "message"
+    ),
+    "Empty QVAL rows removed for QNAM = TESTVAR"
+  )
+  
+  # Test verbose = "warn" - suppress messages
+  expect_silent(
+    result_warn <- build_qnam(
+      dataset = ae,
+      qnam = "TESTVAR",
+      qlabel = "Test Variable",
+      idvar = "AESEQ",
+      qeval = "INVESTIGATOR",
+      qorig = "CRF",
+      verbose = "warn"
+    )
+  )
+  
+  # Test verbose = "silent" - suppress all output
+  expect_silent(
+    result_silent <- build_qnam(
+      dataset = ae,
+      qnam = "TESTVAR",
+      qlabel = "Test Variable",
+      idvar = "AESEQ",
+      qeval = "INVESTIGATOR",
+      qorig = "CRF",
+      verbose = "silent"
+    )
+  )
+  
+  # Verify all verbose levels return same result
+  result_message <- suppressMessages(
+    build_qnam(
+      dataset = ae,
+      qnam = "TESTVAR",
+      qlabel = "Test Variable",
+      idvar = "AESEQ",
+      qeval = "INVESTIGATOR",
+      qorig = "CRF",
+      verbose = "message"
+    )
+  )
+  
+  expect_equal(result_message, result_warn)
+  expect_equal(result_message, result_silent)
+  
+  # Verify empty strings were actually removed
+  expect_false("" %in% result_message$QVAL)
+  
+  # Test invalid verbose value
+  expect_error(
+    build_qnam(
+      dataset = ae,
+      qnam = "TESTVAR",
+      qlabel = "Test Variable",
+      idvar = "AESEQ",
+      qeval = "INVESTIGATOR",
+      qorig = "CRF",
+      verbose = "invalid"
+    ),
+    "'arg' should be one of"
+  )
+})
\ No newline at end of file

From 3b6ac6dab51a858da6bd6e632f0d17e389e8e542 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Wed, 10 Dec 2025 17:03:55 -0600
Subject: [PATCH 04/12] #109 add verbose to set_variable_labels

---
 R/labels.R                   | 17 +++++++--
 man/set_variable_labels.Rd   | 14 ++++++-
 tests/testthat/test-labels.R | 74 ++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+), 5 deletions(-)

diff --git a/R/labels.R b/R/labels.R
index 7e6cee3..2fdc44a 100644
--- a/R/labels.R
+++ b/R/labels.R
@@ -73,6 +73,12 @@ remove_labels <- function(data) {
 #'   Note: Deprecated in version 0.2.0. The `dataset_name` argument will be removed
 #'   in a future release. Please use `metacore::select_dataset` to subset the
 #'   `metacore` object to obtain metadata for a single dataset.
+#' @param verbose Character string controlling message verbosity. One of:
+#'   \describe{
+#'     \item{`"message"`}{Show both warnings and messages (default)}
+#'     \item{`"warn"`}{Show warnings but suppress messages}
+#'     \item{`"silent"`}{Suppress all warnings and messages}
+#'   }
 #'
 #' @return Dataframe with labels applied
 #' @export
@@ -85,7 +91,8 @@ remove_labels <- function(data) {
 #'         )
 #' dm <- haven::read_xpt(metatools_example("dm.xpt"))
 #' set_variable_labels(dm, mc, dataset_name = "DM")
-set_variable_labels <- function(data, metacore, dataset_name = deprecated()) {
+set_variable_labels <- function(data, metacore, dataset_name = deprecated(),
+                                verbose = c("message", "warn", "silent")) {
    if (is_present(dataset_name)) {
       lifecycle::deprecate_warn(
          when = "0.2.0",
@@ -97,6 +104,8 @@ set_variable_labels <- function(data, metacore, dataset_name = deprecated()) {
       metacore <- make_lone_dataset(metacore, dataset_name)
    }
    verify_DatasetMeta(metacore)
+   
+   verbose <- validate_verbose(verbose)
 
    # Grab out the var names and labels
    var_spec <- metacore$var_spec %>%
@@ -112,12 +121,12 @@ set_variable_labels <- function(data, metacore, dataset_name = deprecated()) {
    in_meta <- ns[which(ns %in% mismatch)]
    in_data <- dns[which(dns %in% mismatch)]
 
-   if (length(in_meta) > 0) {
+   if (length(in_meta) > 0 && should_warn(verbose)) {
       wrn <- paste0("Variables in metadata not in data:\n\t", paste0(in_meta, collapse="\n\t"))
       warning(wrn, call. = FALSE)
    }
 
-   if (length(in_data) > 0) {
+   if (length(in_data) > 0 && should_warn(verbose)) {
       wrn <- paste0("Variables in data not in metadata:\n\t", paste0(in_data, collapse="\n\t"))
       warning(wrn, call. = FALSE)
    }
@@ -136,4 +145,4 @@ set_variable_labels <- function(data, metacore, dataset_name = deprecated()) {
    args = append(list(data), labs)
    do.call(add_labels, args)
 
-}
+}
\ No newline at end of file
diff --git a/man/set_variable_labels.Rd b/man/set_variable_labels.Rd
index 282156a..833bd7e 100644
--- a/man/set_variable_labels.Rd
+++ b/man/set_variable_labels.Rd
@@ -4,7 +4,12 @@
 \alias{set_variable_labels}
 \title{Apply labels to a data frame using a metacore object}
 \usage{
-set_variable_labels(data, metacore, dataset_name = deprecated())
+set_variable_labels(
+  data,
+  metacore,
+  dataset_name = deprecated(),
+  verbose = c("message", "warn", "silent")
+)
 }
 \arguments{
 \item{data}{A dataframe or tibble upon which labels will be applied}
@@ -18,6 +23,13 @@ object provided hasn't already been subsetted.\cr
 Note: Deprecated in version 0.2.0. The \code{dataset_name} argument will be removed
 in a future release. Please use \code{metacore::select_dataset} to subset the
 \code{metacore} object to obtain metadata for a single dataset.}
+
+\item{verbose}{Character string controlling message verbosity. One of:
+\describe{
+\item{\code{"message"}}{Show both warnings and messages (default)}
+\item{\code{"warn"}}{Show warnings but suppress messages}
+\item{\code{"silent"}}{Suppress all warnings and messages}
+}}
 }
 \value{
 Dataframe with labels applied
diff --git a/tests/testthat/test-labels.R b/tests/testthat/test-labels.R
index 3597819..efeb86b 100644
--- a/tests/testthat/test-labels.R
+++ b/tests/testthat/test-labels.R
@@ -132,3 +132,77 @@ test_that("removal_labels works to remvoe all labels", {
 
    expect_error(remove_labels(c(1:10)))
 })
+test_that("set_variable_labels verbose parameter", {
+  load(metacore::metacore_example("pilot_SDTM.rda"))
+  spec <- metacore %>% select_dataset("DM", quiet = TRUE)
+  
+  dm <- haven::read_xpt(metatools_example("dm.xpt"))
+  
+  # Get the actual variables in the metadata
+  meta_vars <- spec$var_spec$variable
+  data_vars <- names(dm)
+  
+  # Find a variable that exists in metadata to remove
+  var_to_remove <- intersect(meta_vars, data_vars)[1]
+  
+  # Create mismatch: add a variable not in metadata, remove a variable that is in metadata
+  dm_mismatch <- dm %>%
+    select(-all_of(var_to_remove)) %>%  # Remove a variable that's in metadata
+    mutate(EXTRAVAR = "test")  # Add a variable not in metadata
+  
+  # Test verbose = "message" or "warn" - should show warnings about mismatches
+  expect_warning(
+    set_variable_labels(dm_mismatch, spec, verbose = "message"),
+    "Variables in"
+  )
+  
+  expect_warning(
+    set_variable_labels(dm_mismatch, spec, verbose = "warn"),
+    "Variables in"
+  )
+  
+  # Test verbose = "silent" - suppress all warnings
+  expect_silent(
+    result_silent <- set_variable_labels(dm_mismatch, spec, verbose = "silent")
+  )
+  
+  # Verify all verbose levels return same result (labels applied the same way)
+  result_message <- suppressWarnings(
+    set_variable_labels(dm_mismatch, spec, verbose = "message")
+  )
+  
+  result_warn <- suppressWarnings(
+    set_variable_labels(dm_mismatch, spec, verbose = "warn")
+  )
+  
+  expect_equal(result_message, result_warn)
+  expect_equal(result_message, result_silent)
+  
+  # Verify labels were actually applied to variables that exist in both
+  common_vars <- intersect(names(result_message), meta_vars)
+  if (length(common_vars) > 0) {
+    expect_true(!is.null(attr(result_message[[common_vars[1]]], "label")))
+  }
+  
+  # Test with perfect match - no warnings with any verbose level
+  # Only keep variables that are in metadata
+  dm_matched <- dm %>% select(all_of(intersect(names(dm), meta_vars)))
+  
+  expect_silent(
+    set_variable_labels(dm_matched, spec, verbose = "message")
+  )
+  
+  expect_silent(
+    set_variable_labels(dm_matched, spec, verbose = "warn")
+  )
+  
+  expect_silent(
+    set_variable_labels(dm_matched, spec, verbose = "silent")
+  )
+  
+  # Test invalid verbose value
+  expect_error(
+    set_variable_labels(dm, spec, verbose = "invalid"),
+    "'arg' should be one of"
+  )
+})
\ No newline at end of file

From 3710fd9158ff0e4f809fc76fcebb0fe9d0269432 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <dickinson.jeffrey@gene.com>
Date: Thu, 11 Dec 2025 11:22:07 -0600
Subject: [PATCH 05/12] Update R/utils.R

Co-authored-by: Liam Hobby <102362579+LiamHobby@users.noreply.github.com>
---
 R/utils.R | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/R/utils.R b/R/utils.R
index b7bc5c8..458897e 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -62,6 +62,14 @@ should_warn <- function(verbose) {
 #' Validate verbose parameter
 #' @param verbose Verbosity level to validate
 #' @noRd
-validate_verbose <- function(verbose) {
-  match.arg(verbose, choices = c("message", "warn", "silent"))
+validate_verbose <- function(verbose, arg = rlang::caller_arg(verbose), call = rlang::caller_env()) {
+   choices <- c("message", "warn", "silent")
+   tryCatch(
+      match.arg(verbose, choices),
+      error = function(e) {
+         cli_abort(c(
+            "x" = "{.arg {arg}} should be one of: {cli::ansi_collapse(choices, last = ', ')}"
+         ), call = call)
+      }
+   )
 }

From 296b6ccda30791cff96c1d6fa555077ec596c635 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Thu, 11 Dec 2025 11:36:52 -0600
Subject: [PATCH 06/12] #109 update tests for invalid choice

---
 tests/testthat/test-build.R  | 2 +-
 tests/testthat/test-labels.R | 2 +-
 tests/testthat/test-supp.R   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R
index 2d49ec8..73075d6 100644
--- a/tests/testthat/test-build.R
+++ b/tests/testthat/test-build.R
@@ -238,7 +238,7 @@ test_that("drop_unspec_vars verbose parameter", {
   # Test invalid verbose value
   expect_error(
     drop_unspec_vars(data, spec, verbose = "invalid"),
-    "'arg' should be one of"
+    "should be one of: message, warn, silent"
   )
 })
 
diff --git a/tests/testthat/test-labels.R b/tests/testthat/test-labels.R
index efeb86b..458f5aa 100644
--- a/tests/testthat/test-labels.R
+++ b/tests/testthat/test-labels.R
@@ -203,6 +203,6 @@ test_that("set_variable_labels verbose parameter", {
   # Test invalid verbose value
   expect_error(
     set_variable_labels(dm, spec, verbose = "invalid"),
-    "'arg' should be one of"
+    "should be one of: message, warn, silent"
   )
 })
\ No newline at end of file
diff --git a/tests/testthat/test-supp.R b/tests/testthat/test-supp.R
index 3d21732..879de7d 100644
--- a/tests/testthat/test-supp.R
+++ b/tests/testthat/test-supp.R
@@ -349,6 +349,6 @@ test_that("build_qnam verbose parameter", {
       qorig = "CRF",
       verbose = "invalid"
     ),
-    "'arg' should be one of"
+    "should be one of: message, warn, silent"
   )
 })
\ No newline at end of file

From 1261f5c80c262945facee7bf1f399be48dc86f44 Mon Sep 17 00:00:00 2001
From: Jeffrey Dickinson <dickinson.jeffrey@gene.com>
Date: Fri, 12 Dec 2025 15:32:10 +0000
Subject: [PATCH 07/12] #109 style files after merge

---
 R/build.R                    |  9 +-------
 R/labels.R                   | 40 ++++++++++++++++++------------------
 R/supp.R                     | 30 +++++++++++++--------------
 R/utils.R                    | 18 ++++++++--------
 tests/testthat/test-build.R  | 18 ++++++++--------
 tests/testthat/test-labels.R | 38 +++++++++++++++++-----------------
 tests/testthat/test-supp.R   | 18 ++++++++--------
 7 files changed, 82 insertions(+), 89 deletions(-)

diff --git a/R/build.R b/R/build.R
index 73a45c5..4fb1793 100644
--- a/R/build.R
+++ b/R/build.R
@@ -73,14 +73,9 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
     metacore <- make_lone_dataset(metacore, dataset_name)
   }
   verify_DatasetMeta(metacore)
-  
+
   verbose <- validate_verbose(verbose)
 
-  # Deprecate KEEP = TRUE
-  keep <- match.arg(as.character(keep), c("TRUE", "FALSE", "ALL", "PREREQUISITE"))
-  if (keep == "TRUE") {
-    cli_warn(paste0(
-      "Setting 'keep' = TRUE has been superseded",
   # Deprecate KEEP = TRUE
   keep <- match.arg(as.character(keep), c("TRUE", "FALSE", "ALL", "PREREQUISITE"))
   if (keep == "TRUE") {
@@ -270,8 +265,6 @@ prepare_join <- function(x, keys, ds_names, verbose = "message") {
                             conflict with {ds_names[[j]]}: {conflicting_cols}."))
         }
       }
-        }
-      }
 
       out[[i]] <- x[[i]] %>%
         select(-any_of(drop_cols))
diff --git a/R/labels.R b/R/labels.R
index cfae70a..e979fef 100644
--- a/R/labels.R
+++ b/R/labels.R
@@ -95,19 +95,19 @@ remove_labels <- function(data) {
 #' set_variable_labels(dm, mc, dataset_name = "DM")
 set_variable_labels <- function(data, metacore, dataset_name = deprecated(),
                                 verbose = c("message", "warn", "silent")) {
-   if (is_present(dataset_name)) {
-      lifecycle::deprecate_warn(
-         when = "0.2.0",
-         what = "check_unique_keys(dataset_name)",
-         details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
+  if (is_present(dataset_name)) {
+    lifecycle::deprecate_warn(
+      when = "0.2.0",
+      what = "check_unique_keys(dataset_name)",
+      details = cli_inform(c("i" = col_red("The {.arg dataset_name} argument will be removed in a future release.
       Please use {.fn metacore::select_dataset} to subset the {.obj metacore} object to obtain
       metadata for a single dataset.")))
-      )
-      metacore <- make_lone_dataset(metacore, dataset_name)
-   }
-   verify_DatasetMeta(metacore)
-   
-   verbose <- validate_verbose(verbose)
+    )
+    metacore <- make_lone_dataset(metacore, dataset_name)
+  }
+  verify_DatasetMeta(metacore)
+
+  verbose <- validate_verbose(verbose)
 
   # Grab out the var names and labels
   var_spec <- metacore$var_spec %>%
@@ -123,15 +123,15 @@ set_variable_labels <- function(data, metacore, dataset_name = deprecated(),
   in_meta <- ns[which(ns %in% mismatch)]
   in_data <- dns[which(dns %in% mismatch)]
 
-   if (length(in_meta) > 0 && should_warn(verbose)) {
-      wrn <- paste0("Variables in metadata not in data:\n\t", paste0(in_meta, collapse="\n\t"))
-      warning(wrn, call. = FALSE)
-   }
+  if (length(in_meta) > 0 && should_warn(verbose)) {
+    wrn <- paste0("Variables in metadata not in data:\n\t", paste0(in_meta, collapse = "\n\t"))
+    warning(wrn, call. = FALSE)
+  }
 
-   if (length(in_data) > 0 && should_warn(verbose)) {
-      wrn <- paste0("Variables in data not in metadata:\n\t", paste0(in_data, collapse="\n\t"))
-      warning(wrn, call. = FALSE)
-   }
+  if (length(in_data) > 0 && should_warn(verbose)) {
+    wrn <- paste0("Variables in data not in metadata:\n\t", paste0(in_data, collapse = "\n\t"))
+    warning(wrn, call. = FALSE)
+  }
 
   # Pick out only the variables which exist in both and build list
   match <- intersect(ns, dns)
@@ -146,4 +146,4 @@ set_variable_labels <- function(data, metacore, dataset_name = deprecated(),
   # Apply the labels to the data
   args <- append(list(data), labs)
   do.call(add_labels, args)
-}
\ No newline at end of file
+}
diff --git a/R/supp.R b/R/supp.R
index b4d0270..8bc7263 100644
--- a/R/supp.R
+++ b/R/supp.R
@@ -17,12 +17,12 @@
 #' @export
 #'
 #'
-build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig, 
+build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig,
                        verbose = c("message", "warn", "silent")) {
-   verbose <- validate_verbose(verbose)
-   
-   # Need QNAM as a variable
-   qval <- as.symbol(qnam)
+  verbose <- validate_verbose(verbose)
+
+  # Need QNAM as a variable
+  qval <- as.symbol(qnam)
 
   # DM won't have an IDVAR so handle that
   if (is.na(idvar) || idvar == "") {
@@ -64,16 +64,16 @@ build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig,
     )
   }
 
-   blank_test <- out %>%
-      pull(QVAL)
-   if(any(blank_test == "")){
-      if (should_message(verbose)) {
-         message(paste0("Empty QVAL rows removed for QNAM = ", unique(out$QNAM)))
-      }
-      out <- out %>%
-         filter(QVAL != "")
-   }
-   out
+  blank_test <- out %>%
+    pull(QVAL)
+  if (any(blank_test == "")) {
+    if (should_message(verbose)) {
+      message(paste0("Empty QVAL rows removed for QNAM = ", unique(out$QNAM)))
+    }
+    out <- out %>%
+      filter(QVAL != "")
+  }
+  out
 }
 
 
diff --git a/R/utils.R b/R/utils.R
index 458897e..91491e2 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -63,13 +63,13 @@ should_warn <- function(verbose) {
 #' @param verbose Verbosity level to validate
 #' @noRd
 validate_verbose <- function(verbose, arg = rlang::caller_arg(verbose), call = rlang::caller_env()) {
-   choices <- c("message", "warn", "silent")
-   tryCatch(
-      match.arg(verbose, choices),
-      error = function(e) {
-         cli_abort(c(
-            "x" = "{.arg {arg}} should be one of: {cli::ansi_collapse(choices, last = ', ')}"
-         ), call = call)
-      }
-   )
+  choices <- c("message", "warn", "silent")
+  tryCatch(
+    match.arg(verbose, choices),
+    error = function(e) {
+      cli_abort(c(
+        "x" = "{.arg {arg}} should be one of: {cli::ansi_collapse(choices, last = ', ')}"
+      ), call = call)
+    }
+  )
 }
diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R
index 73075d6..06aef07 100644
--- a/tests/testthat/test-build.R
+++ b/tests/testthat/test-build.R
@@ -245,29 +245,29 @@ test_that("drop_unspec_vars verbose parameter", {
 test_that("build_from_derived verbose controls prepare_join messages", {
   load(metacore::metacore_example("pilot_ADaM.rda"))
   spec2 <- metacore %>% select_dataset("ADAE", quiet = TRUE)
-  
+
   # Use safetyData datasets and add a conflicting non-key column
   # STUDYID is a key column, so add a different column that will conflict
   ae <- safetyData::sdtm_ae %>%
-    mutate(TESTCOL = "AE_VALUE")  # Add a non-key column
-  
+    mutate(TESTCOL = "AE_VALUE") # Add a non-key column
+
   adsl <- safetyData::adam_adsl %>%
-    mutate(TESTCOL = "ADSL_VALUE")  # Same column with different value
-  
+    mutate(TESTCOL = "ADSL_VALUE") # Same column with different value
+
   ds_list <- list(AE = ae, ADSL = adsl)
-  
+
   # Test that conflicting column messages are shown with verbose = "message"
   expect_message(
     build_from_derived(spec2, ds_list, predecessor_only = FALSE, verbose = "message", keep = "ALL"),
     "Dropping column\\(s\\) from"
   )
-  
+
   # Test that messages are suppressed with verbose = "warn" and "silent"
   expect_silent(
     build_from_derived(spec2, ds_list, predecessor_only = FALSE, verbose = "warn", keep = "ALL")
   )
-  
+
   expect_silent(
     build_from_derived(spec2, ds_list, predecessor_only = FALSE, verbose = "silent", keep = "ALL")
   )
-})
\ No newline at end of file
+})
diff --git a/tests/testthat/test-labels.R b/tests/testthat/test-labels.R
index 9576ffa..723bf83 100644
--- a/tests/testthat/test-labels.R
+++ b/tests/testthat/test-labels.R
@@ -135,74 +135,74 @@ test_that("removal_labels works to remvoe all labels", {
 test_that("set_variable_labels verbose parameter", {
   load(metacore::metacore_example("pilot_SDTM.rda"))
   spec <- metacore %>% select_dataset("DM", quiet = TRUE)
-  
+
   dm <- haven::read_xpt(metatools_example("dm.xpt"))
-  
+
   # Get the actual variables in the metadata
   meta_vars <- spec$var_spec$variable
   data_vars <- names(dm)
-  
+
   # Find a variable that exists in metadata to remove
   var_to_remove <- intersect(meta_vars, data_vars)[1]
-  
+
   # Create mismatch: add a variable not in metadata, remove a variable that is in metadata
   dm_mismatch <- dm %>%
-    select(-all_of(var_to_remove)) %>%  # Remove a variable that's in metadata
-    mutate(EXTRAVAR = "test")  # Add a variable not in metadata
-  
+    select(-all_of(var_to_remove)) %>% # Remove a variable that's in metadata
+    mutate(EXTRAVAR = "test") # Add a variable not in metadata
+
   # Test verbose = "message" or "warn" - should show warnings about mismatches
   expect_warning(
     set_variable_labels(dm_mismatch, spec, verbose = "message"),
     "Variables in"
   )
-  
+
   expect_warning(
     set_variable_labels(dm_mismatch, spec, verbose = "warn"),
     "Variables in"
   )
-  
+
   # Test verbose = "silent" - suppress all warnings
   expect_silent(
     result_silent <- set_variable_labels(dm_mismatch, spec, verbose = "silent")
   )
-  
+
   # Verify all verbose levels return same result (labels applied the same way)
   result_message <- suppressWarnings(
     set_variable_labels(dm_mismatch, spec, verbose = "message")
   )
-  
+
   result_warn <- suppressWarnings(
     set_variable_labels(dm_mismatch, spec, verbose = "warn")
   )
-  
+
   expect_equal(result_message, result_warn)
   expect_equal(result_message, result_silent)
-  
+
   # Verify labels were actually applied to variables that exist in both
   common_vars <- intersect(names(result_message), meta_vars)
   if (length(common_vars) > 0) {
     expect_true(!is.null(attr(result_message[[common_vars[1]]], "label")))
   }
-  
+
   # Test with perfect match - no warnings with any verbose level
   # Only keep variables that are in metadata
   dm_matched <- dm %>% select(all_of(intersect(names(dm), meta_vars)))
-  
+
   expect_silent(
     set_variable_labels(dm_matched, spec, verbose = "message")
   )
-  
+
   expect_silent(
     set_variable_labels(dm_matched, spec, verbose = "warn")
   )
-  
+
   expect_silent(
     set_variable_labels(dm_matched, spec, verbose = "silent")
   )
-  
+
   # Test invalid verbose value
   expect_error(
     set_variable_labels(dm, spec, verbose = "invalid"),
     "should be one of: message, warn, silent"
   )
-})
\ No newline at end of file
+})
diff --git a/tests/testthat/test-supp.R b/tests/testthat/test-supp.R
index 9e644b5..3a6894c 100644
--- a/tests/testthat/test-supp.R
+++ b/tests/testthat/test-supp.R
@@ -300,8 +300,8 @@ test_that("build_qnam verbose parameter", {
   # Create simple test data with a column that will be used as QNAM
   ae <- safetyData::sdtm_ae %>%
     head(10) %>%
-    mutate(TESTVAR = c("", "", "Y", "Y", "N", "", "Y", "N", "Y", ""))  # Some empty strings
-  
+    mutate(TESTVAR = c("", "", "Y", "Y", "N", "", "Y", "N", "Y", "")) # Some empty strings
+
   # Test verbose = "message" (default) - should show message about empty QVAL
   expect_message(
     build_qnam(
@@ -315,7 +315,7 @@ test_that("build_qnam verbose parameter", {
     ),
     "Empty QVAL rows removed for QNAM = TESTVAR"
   )
-  
+
   # Test verbose = "warn" - suppress messages
   expect_silent(
     result_warn <- build_qnam(
@@ -328,7 +328,7 @@ test_that("build_qnam verbose parameter", {
       verbose = "warn"
     )
   )
-  
+
   # Test verbose = "silent" - suppress all output
   expect_silent(
     result_silent <- build_qnam(
@@ -341,7 +341,7 @@ test_that("build_qnam verbose parameter", {
       verbose = "silent"
     )
   )
-  
+
   # Verify all verbose levels return same result
   result_message <- suppressMessages(
     build_qnam(
@@ -354,13 +354,13 @@ test_that("build_qnam verbose parameter", {
       verbose = "message"
     )
   )
-  
+
   expect_equal(result_message, result_warn)
   expect_equal(result_message, result_silent)
-  
+
   # Verify empty strings were actually removed
   expect_false("" %in% result_message$QVAL)
-  
+
   # Test invalid verbose value
   expect_error(
     build_qnam(
@@ -374,4 +374,4 @@ test_that("build_qnam verbose parameter", {
     ),
     "should be one of: message, warn, silent"
   )
-})
\ No newline at end of file
+})

From e2bac18b2c697aba65bf97a942e244cc7097b666 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Fri, 12 Dec 2025 10:03:16 -0600
Subject: [PATCH 08/12] #109 change should_ to check_

---
 R/build.R  | 8 ++++----
 R/labels.R | 4 ++--
 R/supp.R   | 2 +-
 R/utils.R  | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/R/build.R b/R/build.R
index 4fb1793..f03dfa5 100644
--- a/R/build.R
+++ b/R/build.R
@@ -122,7 +122,7 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
     str_to_lower()
   if (!all(ds_names %in% names(ds_list))) {
     unknown <- keep(names(ds_list), ~ !. %in% ds_names)
-    if (length(unknown) > 0 && should_warn(verbose)) {
+    if (length(unknown) > 0 && check_warn(verbose)) {
       warning(paste0("The following dataset(s) have no predecessors and will be ignored:\n"),
         paste0(unknown, collapse = ", "),
         call. = FALSE
@@ -132,7 +132,7 @@ build_from_derived <- function(metacore, ds_list, dataset_name = deprecated(),
       str_to_upper() %>%
       paste0(collapse = ", ")
 
-    if (should_message(verbose)) {
+    if (check_message(verbose)) {
       message(paste0(
         "Not all datasets provided. Only variables from ",
         ds_using,
@@ -260,7 +260,7 @@ prepare_join <- function(x, keys, ds_names, verbose = "message") {
           intersect(colnames(x[[i]]))
         drop_cols <- c(drop_cols, conflicting_cols)
 
-        if (length(conflicting_cols) > 0 && should_message(verbose)) {
+        if (length(conflicting_cols) > 0 && check_message(verbose)) {
           cli_inform(c("i" = "Dropping column(s) from {ds_names[[i]]} due to \
                             conflict with {ds_names[[j]]}: {conflicting_cols}."))
         }
@@ -329,7 +329,7 @@ drop_unspec_vars <- function(dataset, metacore, dataset_name = deprecated(),
   if (length(to_drop) > 0) {
     out <- dataset %>%
       select(-all_of(to_drop))
-    if (should_message(verbose)) {
+    if (check_message(verbose)) {
       message(paste0(
         "The following variable(s) were dropped:\n  ",
         paste0(to_drop, collapse = "\n  ")
diff --git a/R/labels.R b/R/labels.R
index e979fef..6b5f556 100644
--- a/R/labels.R
+++ b/R/labels.R
@@ -123,12 +123,12 @@ set_variable_labels <- function(data, metacore, dataset_name = deprecated(),
   in_meta <- ns[which(ns %in% mismatch)]
   in_data <- dns[which(dns %in% mismatch)]
 
-  if (length(in_meta) > 0 && should_warn(verbose)) {
+  if (length(in_meta) > 0 && check_warn(verbose)) {
     wrn <- paste0("Variables in metadata not in data:\n\t", paste0(in_meta, collapse = "\n\t"))
     warning(wrn, call. = FALSE)
   }
 
-  if (length(in_data) > 0 && should_warn(verbose)) {
+  if (length(in_data) > 0 && check_warn(verbose)) {
     wrn <- paste0("Variables in data not in metadata:\n\t", paste0(in_data, collapse = "\n\t"))
     warning(wrn, call. = FALSE)
   }
diff --git a/R/supp.R b/R/supp.R
index 8bc7263..98017a3 100644
--- a/R/supp.R
+++ b/R/supp.R
@@ -67,7 +67,7 @@ build_qnam <- function(dataset, qnam, qlabel, idvar, qeval, qorig,
   blank_test <- out %>%
     pull(QVAL)
   if (any(blank_test == "")) {
-    if (should_message(verbose)) {
+    if (check_message(verbose)) {
       message(paste0("Empty QVAL rows removed for QNAM = ", unique(out$QNAM)))
     }
     out <- out %>%
diff --git a/R/utils.R b/R/utils.R
index 91491e2..216ec89 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -48,14 +48,14 @@ make_lone_dataset <- function(metacore, dataset_name) {
 #' Check if messages should be displayed
 #' @param verbose Verbosity level
 #' @noRd
-should_message <- function(verbose) {
+check_message <- function(verbose) {
   verbose == "message"
 }
 
 #' Check if warnings should be displayed
 #' @param verbose Verbosity level
 #' @noRd
-should_warn <- function(verbose) {
+check_warn <- function(verbose) {
   verbose %in% c("message", "warn")
 }
 

From 9fd5a44b48d61425ed10823683998302b49aafd9 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Fri, 12 Dec 2025 10:08:57 -0600
Subject: [PATCH 09/12] #109 add details to test add_variables

---
 tests/testthat/test-build.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test-build.R b/tests/testthat/test-build.R
index 06aef07..7bd365f 100644
--- a/tests/testthat/test-build.R
+++ b/tests/testthat/test-build.R
@@ -175,7 +175,7 @@ test_that("build_from_derived", {
 })
 
 
-test_that("add_variables", {
+test_that("add_variables add missing variables to the metacore object", {
   load(metacore::metacore_example("pilot_ADaM.rda"))
   spec <- metacore %>% select_dataset("ADSL", quiet = TRUE)
   data <- haven::read_xpt(metatools_example("adsl.xpt")) %>%

From 38356ad304c621c3a8abd1e44b7506ecbed17aec Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Tue, 16 Dec 2025 12:26:28 -0600
Subject: [PATCH 10/12] #109 update tests for verbose argument

---
 tests/testthat/test-labels.R | 41 ++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/tests/testthat/test-labels.R b/tests/testthat/test-labels.R
index ea5b1c0..a28bd05 100644
--- a/tests/testthat/test-labels.R
+++ b/tests/testthat/test-labels.R
@@ -122,25 +122,58 @@ test_that("set_variable_labels respects verbose parameter", {
   spec <- metacore |> select_dataset("DM", quiet = TRUE)
   dm <- haven::read_xpt(metatools_example("dm.xpt"))
   
-  # Create data with mismatch
+  # Create data with mismatch to trigger warnings
   dm_mismatch <- dm |> 
-    select(-RACE) |>  # Remove a variable in metadata
-    mutate(EXTRAVAR = "test")  # Add variable not in metadata
+    select(-RACE) |>  
+    mutate(EXTRAVAR = "test")
   
   # verbose = "silent" suppresses warnings
   expect_silent(
     set_variable_labels(dm_mismatch, spec, verbose = "silent")
   )
   
-  # verbose = "message" or "warn" show warnings
+  # verbose = "message" shows warnings  
   expect_warning(
     set_variable_labels(dm_mismatch, spec, verbose = "message"),
     "Variables in"
   )
   
+  # verbose = "warn" shows warnings
+  expect_warning(
+    set_variable_labels(dm_mismatch, spec, verbose = "warn"),
+    "Variables in"
+  )
+  
   # Invalid verbose value errors
   expect_error(
     set_variable_labels(dm, spec, verbose = "invalid"),
     "should be one of"
   )
+})
+
+test_that("remove_labels removes labels properly", {
+  # Add labels first
+  x <- mtcars |>
+    add_labels(
+      mpg = "Miles Per Gallon",
+      cyl = "Cylinders"
+    )
+  
+  # Verify labels exist
+  expect_equal(attr(x$mpg, "label"), "Miles Per Gallon")
+  expect_equal(attr(x$cyl, "label"), "Cylinders")
+  
+  # Remove labels
+  x_no_labels <- remove_labels(x)
+  
+  # Verify labels are gone
+  expect_null(attr(x_no_labels$mpg, "label"))
+  expect_null(attr(x_no_labels$cyl, "label"))
+})
+
+test_that("remove_labels errors on invalid input", {
+  expect_error(
+    remove_labels("not a dataframe"),
+    "Labels must be removed from a data.frame or tibble"
+  )
 })
\ No newline at end of file

From d53fd679410713a4062d17eab1022a6f56e78ea8 Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Tue, 16 Dec 2025 12:29:41 -0600
Subject: [PATCH 11/12] #109 run styler

---
 tests/testthat/test-labels.R | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/testthat/test-labels.R b/tests/testthat/test-labels.R
index a28bd05..ab039d6 100644
--- a/tests/testthat/test-labels.R
+++ b/tests/testthat/test-labels.R
@@ -121,29 +121,29 @@ test_that("set_variable_labels respects verbose parameter", {
   load(metacore::metacore_example("pilot_SDTM.rda"))
   spec <- metacore |> select_dataset("DM", quiet = TRUE)
   dm <- haven::read_xpt(metatools_example("dm.xpt"))
-  
+
   # Create data with mismatch to trigger warnings
-  dm_mismatch <- dm |> 
-    select(-RACE) |>  
+  dm_mismatch <- dm |>
+    select(-RACE) |>
     mutate(EXTRAVAR = "test")
-  
+
   # verbose = "silent" suppresses warnings
   expect_silent(
     set_variable_labels(dm_mismatch, spec, verbose = "silent")
   )
-  
-  # verbose = "message" shows warnings  
+
+  # verbose = "message" shows warnings
   expect_warning(
     set_variable_labels(dm_mismatch, spec, verbose = "message"),
     "Variables in"
   )
-  
+
   # verbose = "warn" shows warnings
   expect_warning(
     set_variable_labels(dm_mismatch, spec, verbose = "warn"),
     "Variables in"
   )
-  
+
   # Invalid verbose value errors
   expect_error(
     set_variable_labels(dm, spec, verbose = "invalid"),
@@ -158,14 +158,14 @@ test_that("remove_labels removes labels properly", {
       mpg = "Miles Per Gallon",
       cyl = "Cylinders"
     )
-  
+
   # Verify labels exist
   expect_equal(attr(x$mpg, "label"), "Miles Per Gallon")
   expect_equal(attr(x$cyl, "label"), "Cylinders")
-  
+
   # Remove labels
   x_no_labels <- remove_labels(x)
-  
+
   # Verify labels are gone
   expect_null(attr(x_no_labels$mpg, "label"))
   expect_null(attr(x_no_labels$cyl, "label"))
@@ -176,4 +176,4 @@ test_that("remove_labels errors on invalid input", {
     remove_labels("not a dataframe"),
     "Labels must be removed from a data.frame or tibble"
   )
-})
\ No newline at end of file
+})

From 580a83d9b01afebb496de30d982736c99f19dd6d Mon Sep 17 00:00:00 2001
From: Jeff Dickinson <jeff.dickinson@navitaslifesciences.com>
Date: Tue, 16 Dec 2025 13:24:43 -0600
Subject: [PATCH 12/12] Update test comments for labels

---
 tests/testthat/test-labels.R | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tests/testthat/test-labels.R b/tests/testthat/test-labels.R
index ab039d6..dbe9ea0 100644
--- a/tests/testthat/test-labels.R
+++ b/tests/testthat/test-labels.R
@@ -58,7 +58,8 @@ mc <- suppressWarnings(
   )
 )
 
-test_that("Check that add_labels applies labels properly", {
+# add_labels() tests ----
+test_that("add_labels applies labels properly", {
   x <- mtcars %>%
     add_labels(
       mpg = "Miles Per Gallon",
@@ -69,14 +70,15 @@ test_that("Check that add_labels applies labels properly", {
   expect_equal(attr(x$cyl, "label"), "Cylinders")
 })
 
-test_that("Check that add_labels errors properly", {
+test_that("add_labels errors on invalid input", {
   expect_error(add_labels(TRUE, x = "label"))
   expect_error(add_labels(mtcars, "label"))
   expect_error(add_labels(mtcars, bad = "label"))
   expect_error(add_labels(mtcars, mpg = 1))
 })
 
-test_that("set_variable_labels applies labels properly", {
+# set_variable_labels() tests ----
+test_that("set_variable_labels applies labels from metacore properly", {
   # Load in the metacore test object and example data
   suppressMessages(
     mc <- metacore::spec_to_metacore(metacore::metacore_example("p21_mock.xlsx"), quiet = TRUE) %>%
@@ -94,7 +96,7 @@ test_that("set_variable_labels applies labels properly", {
   expect_equal(labs, mc$var_spec$label)
 })
 
-test_that("set_variable_labels raises warnings properly", {
+test_that("set_variable_labels warns on variable mismatches", {
   # This is metadata for the dplyr::starwars dataset
   mc <- suppressWarnings(
     suppressMessages(
@@ -102,19 +104,19 @@ test_that("set_variable_labels raises warnings properly", {
     )
   ) %>% select_dataset("Starwars", quiet = TRUE)
 
+  # Variables in data not in metadata
   starwars_short2 <- starwars_short
   starwars_short2$new_var <- ""
-
-  # Variables in data not in metadata
   expect_warning(set_variable_labels(starwars_short2, mc))
 
-  mc <- suppressWarnings(
+  # Variables in metadata not in data
+  mc_subset <- suppressWarnings(
     suppressMessages(
       metacore::metacore(ds_spec, ds_vars[1:4, ], var_spec[1:4, ], value_spec, derivations, code_id) %>%
         metacore::select_dataset("Starwars", quiet = TRUE)
     )
   )
-  expect_warning(set_variable_labels(starwars_short, mc))
+  expect_warning(set_variable_labels(starwars_short, mc_subset))
 })
 
 test_that("set_variable_labels respects verbose parameter", {
@@ -151,6 +153,7 @@ test_that("set_variable_labels respects verbose parameter", {
   )
 })
 
+# remove_labels() tests ----
 test_that("remove_labels removes labels properly", {
   # Add labels first
   x <- mtcars |>