From 70fb4d7561d09050d1f10716709e79a6fe4c52d9 Mon Sep 17 00:00:00 2001 From: selkamand <73202525+selkamand@users.noreply.github.com> Date: Thu, 12 Feb 2026 08:33:47 +1100 Subject: [PATCH] Improve missing threshold validation in assertions --- NEWS.md | 2 ++ R/assert_compare.R | 39 ++++++++++++++++++++++++++++ R/assert_length.R | 5 ++++ R/is_functions.R | 2 +- tests/testthat/test-assert_compare.R | 20 ++++++++++++++ tests/testthat/test-assert_length.R | 10 +++++++ tests/testthat/test-is_functions.R | 4 +++ 7 files changed, 81 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 02099c0..1d0ec5e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -16,6 +16,8 @@ * Improved missing-value errors for numeric comparison assertions (e.g., `assert_greater_than()` with `NaN`) +* Improved threshold validation so missing `length`, `minimum`, and `maximum` arguments now return informative assertion errors instead of base R missing-value failures + * Improved `assert_numeric()` error messaging for non-numeric matrices/arrays # assertions 0.2.0 diff --git a/R/assert_compare.R b/R/assert_compare.R index 1541b89..ca835db 100644 --- a/R/assert_compare.R +++ b/R/assert_compare.R @@ -1,3 +1,36 @@ +# Comparison threshold checks --------------------------------------------- + +validate_minimum <- assert_create(function(x, minimum){ + if(!is.numeric(minimum)) return("'minimum' must be numeric") + if(length(minimum) != 1) return("'minimum' must be a single number") + if(is.na(minimum) || is.nan(minimum)) return("'minimum' must not be missing") + TRUE +}) + +validate_maximum <- assert_create(function(x, maximum){ + if(!is.numeric(maximum)) return("'maximum' must be numeric") + if(length(maximum) != 1) return("'maximum' must be a single number") + if(is.na(maximum) || is.nan(maximum)) return("'maximum' must not be missing") + TRUE +}) + +validate_between_thresholds <- assert_create(function(x, minimum, maximum, inclusive = TRUE){ + if(!is.numeric(minimum)) return("'minimum' must be numeric") + if(length(minimum) != 1) return("'minimum' must be a single number") + if(is.na(minimum) || is.nan(minimum)) return("'minimum' must not be missing") + + if(!is.numeric(maximum)) return("'maximum' must be numeric") + if(length(maximum) != 1) return("'maximum' must be a single number") + if(is.na(maximum) || is.nan(maximum)) return("'maximum' must not be missing") + + if(!is.logical(inclusive)) return("'inclusive' must be logical") + if(length(inclusive) != 1) return("'inclusive' must be a single logical value") + if(is.na(inclusive)) return("'inclusive' must not be missing") + + TRUE +}) + + #' Assert input is greater than a specified minimum value #' #' Assert all elements in a numeric vector/matrix are above some minimum value. @@ -28,6 +61,7 @@ assert_all_greater_than <- assert_create_chain( assert_numeric, assert_no_missing, + validate_minimum, assert_create( is_greater_than, default_error_msg = "{.strong {arg_name}} must {ifelse(length(arg_value) > 1, 'all ', '')}be {.strong greater than} `{.strong {minimum}}`." @@ -87,6 +121,7 @@ assert_greater_than <- assert_create_chain( assert_all_greater_than_or_equal_to <- assert_create_chain( assert_numeric, assert_no_missing, + validate_minimum, assert_create( is_greater_than_or_equal_to, default_error_msg = "{.strong {arg_name}} must {ifelse(length(arg_value) > 1, 'all ', '')}be {.strong greater than or equal to} `{.strong {minimum}}`." @@ -205,6 +240,7 @@ assert_equal <- assert_create(is_equal, default_error_msg = "{.strong {arg_name} assert_all_less_than <- assert_create_chain( assert_numeric, assert_no_missing, + validate_maximum, assert_create( is_less_than, default_error_msg = "{.strong {arg_name}} must {ifelse(length(arg_value) > 1, 'all ', '')}be {.strong less than} `{.strong {maximum}}`." @@ -264,6 +300,7 @@ assert_less_than <- assert_create_chain( assert_all_less_than_or_equal_to <- assert_create_chain( assert_numeric, assert_no_missing, + validate_maximum, assert_create( is_less_than_or_equal_to, default_error_msg = "{.strong {arg_name}} must {ifelse(length(arg_value) > 1, 'all ', '')}be {.strong less than or equal to} `{.strong {maximum}}`." @@ -321,6 +358,7 @@ assert_less_than_or_equal_to <- assert_create_chain( assert_all_between <- assert_create_chain( assert_numeric, assert_no_missing, + validate_between_thresholds, assert_create( is_between, default_error_msg = "{.strong {arg_name}} must {ifelse(length(arg_value) > 1, 'all ', '')}be {.strong between} {.strong {minimum}} and {.strong {maximum}} {ifelse(inclusive, '(inclusive)', '(exclusive)')}." @@ -353,6 +391,7 @@ assert_all_between <- assert_create_chain( assert_between <- assert_create_chain( assert_number, assert_no_missing, + validate_between_thresholds, assert_create( is_between, default_error_msg = "{.strong {arg_name}} must be {.strong between} {.strong {minimum}} and {.strong {maximum}} {ifelse(inclusive, '(inclusive)', '(exclusive)')}." diff --git a/R/assert_length.R b/R/assert_length.R index dda9909..725c4b7 100644 --- a/R/assert_length.R +++ b/R/assert_length.R @@ -16,6 +16,7 @@ assert_length <- assert_create( func = function(x, length) { if(!is.numeric(length)) return("'length' must be numeric") if(length(length) != 1) return("'length' must be a single number") + if(is.na(length) || is.nan(length)) return("'length' must not be missing") if(!is_whole_number(length)) return("'length' must be a whole number") if(length < 0) return("'length' must be non-negative") @@ -35,6 +36,7 @@ assert_length_greater_than <- assert_create( func = function(x, length) { if(!is.numeric(length)) return("'length' must be numeric") if(length(length) != 1) return("'length' must be a single number") + if(is.na(length) || is.nan(length)) return("'length' must not be missing") if(!is_whole_number(length)) return("'length' must be a whole number") if(length < 0) return("'length' must be non-negative") @@ -54,6 +56,7 @@ assert_length_greater_than_or_equal_to <- assert_create( func = function(x, length) { if(!is.numeric(length)) return("'length' must be numeric") if(length(length) != 1) return("'length' must be a single number") + if(is.na(length) || is.nan(length)) return("'length' must not be missing") if(!is_whole_number(length)) return("'length' must be a whole number") if(length < 0) return("'length' must be non-negative") @@ -73,6 +76,7 @@ assert_length_less_than <- assert_create( func = function(x, length) { if(!is.numeric(length)) return("'length' must be numeric") if(length(length) != 1) return("'length' must be a single number") + if(is.na(length) || is.nan(length)) return("'length' must not be missing") if(!is_whole_number(length)) return("'length' must be a whole number") if(length < 0) return("'length' must be non-negative") @@ -92,6 +96,7 @@ assert_length_less_than_or_equal_to <- assert_create( func = function(x, length) { if(!is.numeric(length)) return("'length' must be numeric") if(length(length) != 1) return("'length' must be a single number") + if(is.na(length) || is.nan(length)) return("'length' must not be missing") if(!is_whole_number(length)) return("'length' must be a whole number") if(length < 0) return("'length' must be non-negative") diff --git a/R/is_functions.R b/R/is_functions.R index c0fa8db..2f7af2b 100644 --- a/R/is_functions.R +++ b/R/is_functions.R @@ -161,7 +161,7 @@ is_reactive <- function(x){ } is_whole_number <- function(x){ - return(x%%1==0) + is.numeric(x) && length(x) == 1 && !is.na(x) && is.finite(x) && x%%1 == 0 } is_all_finite <- function(x){ diff --git a/tests/testthat/test-assert_compare.R b/tests/testthat/test-assert_compare.R index d7c8dec..787f085 100644 --- a/tests/testthat/test-assert_compare.R +++ b/tests/testthat/test-assert_compare.R @@ -213,6 +213,26 @@ cli::test_that_cli("numeric comparison assertions reject NaN with a missing-valu expect_error(assert_all_between(c(1, NaN), 0, 2), "must have no missing values", fixed = FALSE) }) + +cli::test_that_cli("numeric comparison assertions validate missing threshold values", config = "plain", { + expect_error(assert_greater_than(5, NA_real_), "'minimum' must not be missing", fixed = TRUE) + expect_error(assert_all_greater_than(c(5, 6), NaN), "'minimum' must not be missing", fixed = TRUE) + + expect_error(assert_greater_than_or_equal_to(5, NA_real_), "'minimum' must not be missing", fixed = TRUE) + expect_error(assert_all_greater_than_or_equal_to(c(5, 6), NaN), "'minimum' must not be missing", fixed = TRUE) + + expect_error(assert_less_than(5, NA_real_), "'maximum' must not be missing", fixed = TRUE) + expect_error(assert_all_less_than(c(5, 6), NaN), "'maximum' must not be missing", fixed = TRUE) + + expect_error(assert_less_than_or_equal_to(5, NA_real_), "'maximum' must not be missing", fixed = TRUE) + expect_error(assert_all_less_than_or_equal_to(c(5, 6), NaN), "'maximum' must not be missing", fixed = TRUE) + + expect_error(assert_between(5, NA_real_, 6), "'minimum' must not be missing", fixed = TRUE) + expect_error(assert_between(5, 1, NaN), "'maximum' must not be missing", fixed = TRUE) + expect_error(assert_all_between(c(2, 3), NA_real_, 4), "'minimum' must not be missing", fixed = TRUE) + expect_error(assert_all_between(c(2, 3), 1, NaN), "'maximum' must not be missing", fixed = TRUE) +}) + cli::test_that_cli("assert_identical() works", config = "plain", { # Passes diff --git a/tests/testthat/test-assert_length.R b/tests/testthat/test-assert_length.R index d12a435..a40387e 100644 --- a/tests/testthat/test-assert_length.R +++ b/tests/testthat/test-assert_length.R @@ -13,6 +13,8 @@ cli::test_that_cli("assert_length() works", configs = "plain", { expect_error(assert_length(1:3, "3"), "'length' must be numeric", fixed = TRUE) expect_error(assert_length(1:3, c(2, 3)), "'length' must be a single number", fixed = TRUE) expect_error(assert_length(1:3, 2.5), "'length' must be a whole number", fixed = TRUE) + expect_error(assert_length(1:3, NA_real_), "'length' must not be missing", fixed = TRUE) + expect_error(assert_length(1:3, NaN), "'length' must not be missing", fixed = TRUE) expect_error(assert_length(1:3, -1), "'length' must be non-negative", fixed = TRUE) # Error messages use variable name @@ -36,6 +38,8 @@ cli::test_that_cli("assert_length_greater_than() works", configs = "plain", { expect_error(assert_length_greater_than(1:3, "3"), "'length' must be numeric", fixed = TRUE) expect_error(assert_length_greater_than(1:3, c(2, 3)), "'length' must be a single number", fixed = TRUE) expect_error(assert_length_greater_than(1:3, 2.5), "'length' must be a whole number", fixed = TRUE) + expect_error(assert_length_greater_than(1:3, NA_real_), "'length' must not be missing", fixed = TRUE) + expect_error(assert_length_greater_than(1:3, NaN), "'length' must not be missing", fixed = TRUE) expect_error(assert_length_greater_than(1:3, -1), "'length' must be non-negative", fixed = TRUE) }) @@ -52,6 +56,8 @@ cli::test_that_cli("assert_length_greater_than_or_equal_to() works", configs = " expect_error(assert_length_greater_than_or_equal_to(1:3, "3"), "'length' must be numeric", fixed = TRUE) expect_error(assert_length_greater_than_or_equal_to(1:3, c(2, 3)), "'length' must be a single number", fixed = TRUE) expect_error(assert_length_greater_than_or_equal_to(1:3, 2.5), "'length' must be a whole number", fixed = TRUE) + expect_error(assert_length_greater_than_or_equal_to(1:3, NA_real_), "'length' must not be missing", fixed = TRUE) + expect_error(assert_length_greater_than_or_equal_to(1:3, NaN), "'length' must not be missing", fixed = TRUE) expect_error(assert_length_greater_than_or_equal_to(1:3, -1), "'length' must be non-negative", fixed = TRUE) }) @@ -68,6 +74,8 @@ cli::test_that_cli("assert_length_less_than() works", configs = "plain", { expect_error(assert_length_less_than(1:3, "3"), "'length' must be numeric", fixed = TRUE) expect_error(assert_length_less_than(1:3, c(2, 3)), "'length' must be a single number", fixed = TRUE) expect_error(assert_length_less_than(1:3, 2.5), "'length' must be a whole number", fixed = TRUE) + expect_error(assert_length_less_than(1:3, NA_real_), "'length' must not be missing", fixed = TRUE) + expect_error(assert_length_less_than(1:3, NaN), "'length' must not be missing", fixed = TRUE) expect_error(assert_length_less_than(1:3, -1), "'length' must be non-negative", fixed = TRUE) }) @@ -84,5 +92,7 @@ cli::test_that_cli("assert_length_less_than_or_equal_to() works", configs = "pla expect_error(assert_length_less_than_or_equal_to(1:3, "3"), "'length' must be numeric", fixed = TRUE) expect_error(assert_length_less_than_or_equal_to(1:3, c(2, 3)), "'length' must be a single number", fixed = TRUE) expect_error(assert_length_less_than_or_equal_to(1:3, 2.5), "'length' must be a whole number", fixed = TRUE) + expect_error(assert_length_less_than_or_equal_to(1:3, NA_real_), "'length' must not be missing", fixed = TRUE) + expect_error(assert_length_less_than_or_equal_to(1:3, NaN), "'length' must not be missing", fixed = TRUE) expect_error(assert_length_less_than_or_equal_to(1:3, -1), "'length' must be non-negative", fixed = TRUE) }) diff --git a/tests/testthat/test-is_functions.R b/tests/testthat/test-is_functions.R index 21d3f32..41413d0 100644 --- a/tests/testthat/test-is_functions.R +++ b/tests/testthat/test-is_functions.R @@ -113,6 +113,10 @@ test_that("is_whole_number works as expected", { expect_false(is_whole_number(2.5)) expect_true(is_whole_number(0)) expect_true(is_whole_number(-3)) + expect_false(is_whole_number(NA_real_)) + expect_false(is_whole_number(NaN)) + expect_false(is_whole_number(Inf)) + expect_false(is_whole_number(c(1, 2))) }) # Test `is_connection` (requires DBI)