Skip to content

Commit b3460d4

Browse files
author
Liam Hobby
committed
Overhaul messaged, warnings, and error handling
1 parent cf32066 commit b3460d4

9 files changed

Lines changed: 154 additions & 153 deletions

File tree

NAMESPACE

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ importFrom(cli,cli_rule)
4444
importFrom(cli,cli_text)
4545
importFrom(cli,cli_warn)
4646
importFrom(cli,col_red)
47+
importFrom(cli,qty)
4748
importFrom(dplyr,across)
4849
importFrom(dplyr,anti_join)
4950
importFrom(dplyr,arrange)
@@ -93,6 +94,7 @@ importFrom(rlang,enexpr)
9394
importFrom(rlang,expr)
9495
importFrom(rlang,inherits_only)
9596
importFrom(rlang,prim_name)
97+
importFrom(rlang,set_names)
9698
importFrom(rlang,sym)
9799
importFrom(stats,na.omit)
98100
importFrom(stats,var)

R/checks.R

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ check_inconsistent_formats <- function(metacore){
5757
#' @importFrom dplyr across
5858
basic_check <- function(col_to_check, metacore){
5959
if(!is_metacore(metacore)){
60-
stop("Expects a metacore object", call. = FALSE)
60+
cli_abort("Expects a metacore object", call. = FALSE)
6161
}
6262

6363
report_df <- metacore$var_spec %>%
@@ -73,10 +73,10 @@ basic_check <- function(col_to_check, metacore){
7373
select(variable = var1, everything())
7474

7575
if(nrow(report_df) > 0){
76-
message(str_glue("Mismatch {as_label(enexpr(col_to_check))}s detected"))
76+
cli_warn(str_glue("Mismatch {as_label(enexpr(col_to_check))}s detected"))
7777
return(report_df)
7878
} else {
79-
message(str_glue("No mismatch {as_label(enexpr(col_to_check))}s detected"))
79+
cli_inform(str_glue("No mismatch {as_label(enexpr(col_to_check))}s detected"))
8080
}
8181
}
8282

R/metacore.R

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ MetaCore_validate <- function() {
121121
nrow(private$.derivations) == 0 &
122122
nrow(private$.codelist) == 0 &
123123
nrow(private$.supp) == 0 ){
124-
warning("Other checks were not preformed, because all datasets are empty",
124+
cli_warn("Other checks were not preformed, because all datasets are empty",
125125
call. = FALSE)
126126
} else {
127127
check_columns(private$.ds_spec,
@@ -144,7 +144,7 @@ MetaCore_validate <- function() {
144144
}
145145

146146
} else {
147-
warning("Other checks were not preformed, because column names were incorrect",
147+
cli_warn("Other checks were not preformed, because column names were incorrect",
148148
call. = FALSE)
149149
}
150150
}
@@ -168,7 +168,7 @@ readonly <- function(name) {
168168
if (missing(value)) {
169169
private[[paste0(".", name)]]
170170
} else {
171-
stop(paste0(name, " is read only"), call. = FALSE)
171+
cli_abort("{name} is read only", call. = FALSE)
172172
}
173173
}
174174
attributes(inside) <- list(name = name)
@@ -182,7 +182,7 @@ MetaCore_filter <- function(value) {
182182

183183
private$.ds_spec <- private$.ds_spec %>% filter(dataset == value)
184184
if(nrow(private$.ds_spec) == 0){
185-
stop(paste0(value, " is not a dataset in the metacore object", call. = FALSE))
185+
cli_abort("{value} is not a dataset in the metacore object", call. = FALSE)
186186
}
187187
private$.ds_vars <- private$.ds_vars %>% filter(dataset == value)
188188
private$.value_spec <- private$.value_spec %>% filter(dataset == value)
@@ -443,7 +443,7 @@ get_control_term <- function(metacode, variable, dataset = NULL){
443443
dataset_val <- ifelse(str_detect(as_label(enexpr(dataset)), "\""),
444444
as_name(dataset), as_label(enexpr(dataset))) # to make the filter more explicit
445445
if(!var_str %in% metacode$value_spec$variable){
446-
stop(paste0(var_str, " not found in the value_spec table. Please check the variable name"))
446+
cli_abort("{var_str} not found in the value_spec table. Please check the variable name")
447447
}
448448
if(dataset_val == "NULL"){
449449
var_code_id <- metacode$value_spec %>%
@@ -454,21 +454,21 @@ get_control_term <- function(metacode, variable, dataset = NULL){
454454
subset_data <- metacode$value_spec %>%
455455
filter(dataset == dataset_val)
456456
if(nrow(subset_data) == 0){
457-
stop(paste0(dataset_val, " not found in the value_spec table. Please check the dataset name"))
457+
cli_abort("{dataset_val} not found in the value_spec table. Please check the dataset name")
458458
}
459459
var_code_id <- subset_data %>%
460460
filter(variable == var_str) %>%
461461
pull(code_id) %>%
462462
unique()
463463
}
464464
if(length(var_code_id) > 1){
465-
stop(paste0(var_str, " does not have a unique control term, consider spcificing a dataset"))
465+
cli_abort("{var_str} does not have a unique control term, consider spcificing a dataset")
466466
}
467467
ct <- metacode$codelist %>%
468468
filter(code_id == var_code_id) %>%
469469
pull(codes)
470470
if(length(ct) == 0){
471-
message(paste0(var_str, " has no control terminology"))
471+
cli_inform("{var_str} has no control terminology")
472472
} else {
473473
return(ct[[1]])
474474
}
@@ -500,7 +500,7 @@ get_keys <- function(metacode, dataset){
500500
subset_data <- metacode$ds_vars %>%
501501
filter(dataset == dataset_val)
502502
if(nrow(subset_data) == 0){
503-
stop(paste0(dataset_val, " not found in the ds_vars table. Please check the dataset name"))
503+
cli_abort("{dataset_val} not found in the ds_vars table. Please check the dataset name")
504504
}
505505

506506
keys <- subset_data %>%
@@ -554,9 +554,9 @@ load_metacore <- function(path = NULL) {
554554
if (is.null(path)) {
555555
rdss <- list.files(".", ".rds")
556556
if (length(rdss) == 0) {
557-
stop("please supply path to metacore object ending with extension .rds", call. = FALSE)
557+
cli_abort("please supply path to metacore object ending with extension .rds", call. = FALSE)
558558
} else {
559-
stop("metacore object path required, did you mean:",
559+
cli_abort("metacore object path required, did you mean:",
560560
paste(" ", rdss, sep = "\n "), call. = FALSE)
561561
}
562562
}

R/spec_builder.R

Lines changed: 52 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ spec_to_metacore <- function(path, quiet = FALSE, where_sep_sheet = TRUE){
2929
out<- suppressWarnings(metacore(ds_spec, ds_vars, var_spec, value_spec, derivations, codelist = code_list, quiet = quiet))
3030
}
3131
} else {
32-
stop("This specification format is not currently supported. You will need to write your own reader",
32+
cli_abort("This specification format is not currently supported. You will need to write your own reader",
3333
call. = FALSE)
3434
}
3535
out
@@ -48,14 +48,14 @@ spec_to_metacore <- function(path, quiet = FALSE, where_sep_sheet = TRUE){
4848
spec_type <- function(path){
4949
sheets <- excel_sheets(path)
5050
if(!any(sheets %>% str_detect("[D|d]omains?|[D|d]atasets?"))){
51-
stop("File does not contain a Domain/Datasets tab, which is needed. Please either modify the spec document or write a reader (see documentation for more information)",
51+
cli_abort("File does not contain a Domain/Datasets tab, which is needed. Please either modify the spec document or write a reader (see documentation for more information)",
5252
call. = FALSE)
5353
} else if(any(sheets %>% str_detect("ADSL|DM"))){
5454
type <- "by_ds"
5555
} else if(any(sheets %>% str_detect("[V|v]ariables?"))){
5656
type <- "by_type"
5757
} else {
58-
stop("File in an unknown format. Please either modify the spec document or write a reader (see documentation for more information)",
58+
cli_abort("File in an unknown format. Please either modify the spec document or write a reader (see documentation for more information)",
5959
call. = FALSE)
6060
}
6161
type
@@ -102,8 +102,10 @@ spec_type_to_ds_spec <- function(doc, cols = c("dataset" = "[N|n]ame|[D|d]ataset
102102
name_check <- names(cols) %in% c("dataset", "structure", "label") %>%
103103
all()
104104
if(!name_check | is.null(names(cols))){
105-
stop("Supplied column vector must be named using the following names:
106-
'dataset', 'structure', 'label'")
105+
cli_abort(c(
106+
"Supplied column vector must be named using the following names:",
107+
"'dataset', 'structure', 'label'"
108+
))
107109
}
108110
if(!is.null(sheet)){
109111
sheet_ls <- str_subset(names(doc), sheet)
@@ -159,7 +161,7 @@ spec_type_to_ds_vars <- function(doc, cols = c("dataset" = "[D|d]ataset|[D|d]oma
159161

160162
# Testing for names of vectors
161163
if(any(!name_check, !name_check_extra, is.null(names(cols)))){
162-
stop("Supplied column vector must be named using the following names:
164+
cli_abort("Supplied column vector must be named using the following names:
163165
'variable', 'dataset', 'order', 'keep', 'core', 'key_seq', 'supp_flag'")
164166
}
165167
# Subsetting sheets
@@ -226,10 +228,12 @@ spec_type_to_var_spec <- function(doc, cols = c("variable" = "[N|n]ame|[V|v]aria
226228
"type", "dataset", "common", "format") %>%
227229
all()
228230
if(!name_check | is.null(names(cols))){
229-
stop("Supplied column vector must be named using the following names:
230-
'variable', 'length', 'label', 'type', 'dataset', 'common', 'format'
231-
If common is not avaliable it can be excluded and will be automatically filled in.
232-
Additionally, dataset is only used to clarify if information differs by domain")
231+
cli_abort(paste(
232+
"Supplied column vector must be named using the following names:",
233+
"'variable', 'length', 'label', 'type', 'dataset', 'common', 'format'",
234+
"If common is not avaliable it can be excluded and will be automatically filled in.",
235+
"Additionally, dataset is only used to clarify if information differs by domain."
236+
))
233237
}
234238

235239
# Check if sheet is specified
@@ -245,12 +249,12 @@ spec_type_to_var_spec <- function(doc, cols = c("variable" = "[N|n]ame|[V|v]aria
245249
summarise(n = n(), .groups = "drop") %>%
246250
filter(n > 1)
247251
if(nrow(dups) > 0){
248-
dups %>%
249-
pull(variable) %>%
250-
paste(collapse = "\n") %>%
251-
paste0("The following variables are repeated with different metadata for different datasets:\n",
252-
., "\nPlease add 'dataset' = [Name of dataset column] to your named cols vector, to correct for this") %>%
253-
stop(., call. = FALSE)
252+
x <- dups %>% pull(variable)
253+
cli_abort(c(
254+
col_red("The following variables are repeated with different metadata for different datasets:"),
255+
"i" = ansi_collapse(x),
256+
"i" = "Please add 'dataset' = [Name of dataset column] to your named cols vector to correct this."
257+
), call. = FALSE)
254258
}
255259
} else {
256260
if(!"common" %in% names(cols)){
@@ -334,12 +338,12 @@ spec_type_to_value_spec <- function(doc, cols = c("dataset" = "[D|d]ataset|[D|d]
334338
all()
335339

336340
if(!name_check| is.null(names(cols))){
337-
stop("Supplied column vector must be named using the following names:
338-
'dataset', 'variable', 'origin', 'code_id', 'type', 'where', 'sig_dig', 'derivation_id',
339-
'predecessor'
340-
If derivation_id is not avaliable it can be excluded and dataset.variable will be used.
341-
342-
If the where information is on a seperate sheet, put the column with cross ref as where.")
341+
cli_abort(c(
342+
"Supplied column vector must be named using the following names:",
343+
"i" = "'dataset', 'variable', 'origin', 'code_id', 'type', 'where', 'sig_dig', 'derivation_id','predecessor'",
344+
"i" = paste("If derivation_id is not avaliable it can be excluded and dataset.variable will be used.",
345+
"If the where information is on a seperate sheet, put the column with cross ref as where.")
346+
), call = FALSE)
343347
}
344348

345349
# Select a subset of sheets if specified
@@ -401,7 +405,7 @@ spec_type_to_value_spec <- function(doc, cols = c("dataset" = "[D|d]ataset|[D|d]
401405
left_join(where_df, by = c("where" = "id")) %>%
402406
select(-where, where = where_new)
403407
} else if(where_sep_sheet) {
404-
warning("Not able to add where infromation from seperate sheet cause a where column is needed to cross-reference the information",
408+
cli_warn("Not able to add where infromation from seperate sheet cause a where column is needed to cross-reference the information",
405409
call. = FALSE)
406410
}
407411

@@ -467,12 +471,12 @@ spec_type_to_codelist <- function(doc, codelist_cols = c("code_id" = "ID",
467471
"version" = "[V|v]ersion"),
468472
sheets = NULL, simplify = FALSE){
469473
if(is.null(codelist_cols)){
470-
stop("Codelist column names must be provided", call. = FALSE)
474+
cli_abort("Codelist column names must be provided", call. = FALSE)
471475
} else {
472476
name_check <- names(codelist_cols) %in% c("code_id", "name", "code", "decode") %>%
473477
all()
474478
if(!name_check| is.null(names(codelist_cols))){
475-
stop("Supplied column vector for codelist_cols must be named using the following names:
479+
cli_abort("Supplied column vector for codelist_cols must be named using the following names:
476480
'code_id', 'name', 'code', 'decode'",
477481
call. = FALSE
478482
)
@@ -483,7 +487,7 @@ spec_type_to_codelist <- function(doc, codelist_cols = c("code_id" = "ID",
483487
name_check <- names(permitted_val_cols) %in% c("code_id", "name", "code") %>%
484488
all()
485489
if(!name_check){
486-
stop("Supplied column vector for permitted_val_cols must be named using the following names:
490+
cli_abort("Supplied column vector for permitted_val_cols must be named using the following names:
487491
'code_id', 'name', 'code'",
488492
call. = FALSE)
489493
}
@@ -492,10 +496,10 @@ spec_type_to_codelist <- function(doc, codelist_cols = c("code_id" = "ID",
492496
name_check <- names(dict_cols) %in% c("code_id", "name", "dictionary", "version") %>%
493497
all()
494498
if(!name_check){
495-
stop("Supplied column vector for `dict_cols` must be named using the following names:
496-
'code_id', 'name', 'dictionary', 'version',
497-
If a dictionary sheet isn't avaliable set `dict_cols` to NULL",
498-
call. = FALSE)
499+
cli_abort(paste0(
500+
"Supplied column vector for `dict_cols` must be named using the following names:",
501+
"'code_id', 'name', 'dictionary', 'version'. If a dictionary sheet isn't avaliable",
502+
"set `dict_cols` to NULL"), call. = FALSE)
499503
}
500504
}
501505

@@ -574,14 +578,15 @@ spec_type_to_derivations <- function(doc, cols = c("derivation_id" = "ID",
574578
name_check <- names(cols) %in% c("derivation_id", "derivation") %>%
575579
all()
576580
if(!name_check| is.null(names(cols))){
577-
stop("Supplied column vector must be named using the following names:
578-
'derivation_id', 'derivation'")
581+
cli_abort(c(
582+
"Supplied column vector must be named using the following names:",
583+
"'derivation_id', 'derivation'"))
579584
}
580585

581586
name_check <- names(var_cols) %in% c('dataset', 'variable', 'origin', 'predecessor', 'comment') %>%
582587
all()
583588
if(!name_check| is.null(names(var_cols))){
584-
stop("Supplied variable column vector must be named using the following names:
589+
cli_abort("Supplied variable column vector must be named using the following names:
585590
'dataset', 'variable', 'origin', 'predecessor', 'comment'")
586591
}
587592
# Get the predecessor
@@ -683,7 +688,7 @@ create_tbl <- function(doc, cols){
683688
}) %>%
684689
paste0(collapse = "\n") %>%
685690
paste0("Unable to identify a sheet with all columns.\n", . ) %>%
686-
stop(call. = FALSE)
691+
(call. = FALSE)
687692

688693
} else if(length(matches) == 1){
689694
# Check names and write a better warning message if names don't work
@@ -702,26 +707,25 @@ create_tbl <- function(doc, cols){
702707
cols[names(nm_test)] <- cols[names(nm_test)] %>%
703708
paste0("^", ., "$")
704709
} else {
705-
str_c(names(nm_test), " matches ",nm_test, " columns") %>%
706-
str_c(collapse = "\n ") %>%
707-
paste0("Unable to rename the following columns in ", names(matches[1]), ":\n ", .,
708-
"\nPlease check your regular expression ") %>%
709-
stop(call. = FALSE)
710+
x <- str_c(names(nm_test), " matches ",nm_test, " columns")
711+
cli_abort(c(
712+
"Unable to rename the following columns in {names(matches[1])}:",
713+
"i" = ansi_collapse(x),
714+
"i" = "Please check your regular expression"
715+
), call. = FALSE)
710716
}
711717
}
712718

713719
# This needs to be done columnwise to allow for duplicate selection of the same column
714720
select_rename_w_dups(matches[[1]], cols)
715721

716722
} else {
717-
sheets_mats <- matches %>%
718-
names()
719-
paste("Column names are not specific enough to identify a single sheet. The following",
720-
length(sheets_mats),
721-
"match the criteria set:", paste(sheets_mats, collapse = ", ")) %>%
722-
warning(., call. = FALSE)
723-
matches %>%
724-
map(~select_rename_w_dups(., cols))
723+
sheets_mats <- matches %>% names()
724+
cli_warn(c(paste(
725+
"Column names are not specific enough to identify a single sheet."),
726+
"The following {length(sheets_mats)} match the criteria set:"),
727+
ansi_collapse(sheets_mats), call. = FALSE)
728+
matches %>% map(~select_rename_w_dups(., cols))
725729
}
726730
}
727731

@@ -741,7 +745,7 @@ yn_to_tf <- function(x){
741745
} else if(is.logical(x)){
742746
x
743747
} else {
744-
warning("Keep column needs to be True or False, please correct before converting to a Metacore object",
748+
cli_warn("Keep column needs to be True or False, please correct before converting to a Metacore object",
745749
call. = FALSE)
746750
x
747751
}

0 commit comments

Comments
 (0)