Validate chart_type early and improve the error message

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")),

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)
+}

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, 

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)

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"
+  )
+})