diff --git a/DESCRIPTION b/DESCRIPTION index 51dacdc..9fd7d64 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: autospc Title: Automatically Partitioned SPC Charts -Version: 0.0.0.9041 +Version: 0.0.0.9042 Authors@R: c( person("Thomas", "Woodcock", , "woodcock.thomas@gmail.com", role = c("aut", "cre"), comment = c(ORCID = "0000-0002-4735-4856")), diff --git a/R/argument_validation.R b/R/argument_validation.R new file mode 100644 index 0000000..e45d1a2 --- /dev/null +++ b/R/argument_validation.R @@ -0,0 +1,59 @@ + +#' Validate chart_type argument +#' +#' Checks that `chart_type` is a single, non-NULL character string corresponding +#' to a supported SPC chart type. Intended for internal use only. +#' +#' @param chart_type Character scalar specifying chart type. +#' +#' @return Invisibly returns TRUE if valid; otherwise errors. +#' @noRd +validate_chart_type <- function(chart_type) { + + allowed_chart_types <- c("XMR", "MR", "C", "C'", "P", "P'") + + # NULL check + if (is.null(chart_type)) { + + lifecycle::deprecate_stop( + when = "0.0.0.9008", + what = I("chart_type = NULL"), + details = I(paste("Please explicitly pass the desired chart type.", + "Available chart types are: ", + paste(allowed_chart_types, collapse = ", "), + "."))) + } + + # Length check + if (length(chart_type) != 1) { + stop( + "chart_type must have length one. ", + "Available chart types are: ", + paste(allowed_chart_types, collapse = ", "), + ".", + call. = FALSE + ) + } + + # Type check (defensive) + if (!is.character(chart_type)) { + stop( + "chart_type must be a character string.", + call. = FALSE + ) + } + + # Value check + if (!chart_type %in% allowed_chart_types) { + stop( + sprintf( + "Invalid chart_type: '%s'. Available chart types are: %s.", + chart_type, + paste(allowed_chart_types, collapse = ", ") + ), + call. = FALSE + ) + } + + invisible(TRUE) +} diff --git a/R/autospc.R b/R/autospc.R index ce5ee40..4de459c 100644 --- a/R/autospc.R +++ b/R/autospc.R @@ -28,8 +28,8 @@ #' } #' @param n Name of column (passed using tidyselect semantics) to use as #' denominator for P and P' charts. -#' @param chart_type The type of chart you wish to plot. Available options are: -#' "XMR", "MR", "C", "C'", "P", "P'". +#' @param chart_type The type of chart you wish to plot. Must must have length one. +#' Available options are: "XMR", "MR", "C", "C'", "P", "P'". #' #' ## Algorithm Parameters #' Parameters that control behaviour of the algorithm used to re-establish @@ -248,9 +248,8 @@ autospc <- function(data, xType <- preprocessed_vars$xType upper_annotation_sf <- preprocessed_vars$upper_annotation_sf lower_annotation_sf <- preprocessed_vars$lower_annotation_sf - - - # Get control limits + + # Get control limits data <- create_SPC_auto_limits_table( data, chart_type = chart_type, diff --git a/R/preprocess.R b/R/preprocess.R index af1425e..a44533a 100644 --- a/R/preprocess.R +++ b/R/preprocess.R @@ -9,6 +9,8 @@ preprocess_inputs <- function( override_annotation_dist = NULL, override_annotation_dist_P = NULL) { + validate_chart_type(chart_type) + #get title from data if(is.null(title) & "title" %in% colnames(df)) { title <- df$title[1] @@ -29,27 +31,6 @@ preprocess_inputs <- function( "'Date', 'POSIXct', 'numeric' or 'integer' type.")) } - #decide whether the chart is C or P depending on data format if not specified - if(is.null(chart_type)) { - - lifecycle::deprecate_warn( - when = "0.0.0.9008", - what = I("chart_type = NULL"), - details = I("Please explicitly pass the desired chart type") - ) - - if(all(c("x", "y") %in% colnames(df))) { - chart_type <- "C'" - } else if(all(c("x", "n", "y") %in% colnames(df))) { - chart_type <- "P'" - } else { - print(paste0("The data you have input is not in the correct format. ", - "For C charts, data must contain at least columns 'x' and ", - "'y'. For P charts data must contain at least 'x', 'n' and ", - "'y' columns.")) - } - } - if(chart_type == "MR") { mrs <- get_mrs(y = df$y) df <- df %>% dplyr::mutate(y = mrs) diff --git a/man/autospc.Rd b/man/autospc.Rd index e9467be..4da6b90 100644 --- a/man/autospc.Rd +++ b/man/autospc.Rd @@ -85,8 +85,8 @@ P' charts. \item{n}{Name of column (passed using tidyselect semantics) to use as denominator for P and P' charts.} -\item{chart_type}{The type of chart you wish to plot. Available options are: -"XMR", "MR", "C", "C'", "P", "P'". +\item{chart_type}{The type of chart you wish to plot. Must must have length one. +Available options are: "XMR", "MR", "C", "C'", "P", "P'". \subsection{Algorithm Parameters}{ Parameters that control behaviour of the algorithm used to re-establish diff --git a/man/create_SPC_auto_limits_table.Rd b/man/create_SPC_auto_limits_table.Rd index ac879e1..c59961f 100644 --- a/man/create_SPC_auto_limits_table.Rd +++ b/man/create_SPC_auto_limits_table.Rd @@ -36,8 +36,8 @@ For a P or P' chart, must have columns for: \item and optionally, a title and subtitle for the plot. }} -\item{chart_type}{The type of chart you wish to plot. Available options are: -"XMR", "MR", "C", "C'", "P", "P'". +\item{chart_type}{The type of chart you wish to plot. Must must have length one. +Available options are: "XMR", "MR", "C", "C'", "P", "P'". \subsection{Algorithm Parameters}{ Parameters that control behaviour of the algorithm used to re-establish diff --git a/tests/testthat/test-autospc-chart-type.R b/tests/testthat/test-autospc-chart-type.R new file mode 100644 index 0000000..c1d6e19 --- /dev/null +++ b/tests/testthat/test-autospc-chart-type.R @@ -0,0 +1,49 @@ +test_that("autospc errors informatively for invalid chart_type", { + + expect_error( + autospc( + ed_attendances_monthly, + chart_type = "XmR'", + x = month_start, + y = att_all + ), + "Available chart types are" + ) + +}) + +test_that("autospc errors when chart_type is NULL", { + expect_error( + autospc( + ed_attendances_monthly, + chart_type = NULL, + x = month_start, + y = att_all + ), + "pass the desired chart type" + ) +}) + +test_that("autospc errors when chart_type has length > 1", { + expect_error( + autospc( + ed_attendances_monthly, + chart_type = c("XMR", "MR"), + x = month_start, + y = att_all + ), + "length one" + ) +}) + +test_that("autospc errors when chart_type is not character", { + expect_error( + autospc( + ed_attendances_monthly, + chart_type = 1, + x = month_start, + y = att_all + ), + "character" + ) +})