Skip to content

Commit

Permalink
difficult-to-diagnose errors using "difftime" response in a linear mo…
Browse files Browse the repository at this point in the history
…del (#679)
  • Loading branch information
strengejacke committed Feb 5, 2024
1 parent e946088 commit 082f9e8
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 10 deletions.
2 changes: 1 addition & 1 deletion DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Type: Package
Package: performance
Title: Assessment of Regression Models Performance
Version: 0.10.8.11
Version: 0.10.8.12
Authors@R:
c(person(given = "Daniel",
family = "Lüdecke",
Expand Down
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
* `check_outliers()` now properly accept the `percentage_central` argument when
using the `"mcd"` method.

* Fixed edge cases in `check_collinearity()` and `check_outliers()` for models
with response variables of classes `Date`, `POSIXct`, `POSIXlt` or `difftime`.

# performance 0.10.8

## Changes
Expand Down
20 changes: 16 additions & 4 deletions R/check_collinearity.R
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,20 @@ check_collinearity.zerocount <- function(x,

.check_collinearity <- function(x, component, ci = 0.95, verbose = TRUE) {

Check warning on line 406 in R/check_collinearity.R

View workflow job for this annotation

GitHub Actions / lint / lint

file=R/check_collinearity.R,line=406,col=1,[cyclocomp_linter] Reduce the cyclomatic complexity of this function from 43 to at most 40.
v <- insight::get_varcov(x, component = component, verbose = FALSE)

# sanity check
if (is.null(v)) {
if (isTRUE(verbose)) {
insight::format_alert(
paste(
sprintf("Could not extract the variance-covariance matrix for the %s component of the model.", component),
"Please try to run `vcov(model)`, which may help identifying the problem."
)
)
}
return(NULL)
}

term_assign <- .term_assignments(x, component, verbose = verbose)

# any assignment found?
Expand Down Expand Up @@ -432,10 +446,8 @@ check_collinearity.zerocount <- function(x,
if (insight::has_intercept(x)) {
v <- v[-1, -1]
term_assign <- term_assign[-1]
} else {
if (isTRUE(verbose)) {
insight::format_alert("Model has no intercept. VIFs may not be sensible.")
}
} else if (isTRUE(verbose)) {
insight::format_alert("Model has no intercept. VIFs may not be sensible.")
}

f <- insight::find_formula(x)
Expand Down
12 changes: 9 additions & 3 deletions R/check_model.R
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,18 @@ check_model.default <- function(x,
suppressWarnings(.check_assumptions_glm(x, minfo, verbose, ...))
},
error = function(e) {
NULL
e
}
)

if (is.null(ca)) {
insight::format_error(paste0("`check_model()` not implemented for models of class `", class(x)[1], "` yet."))
if (inherits(ca, c("error", "simpleError"))) {
pattern <- "(\n|\\s{2,})"
replacement <- " "
cleaned_string <- gsub(pattern, replacement, ca$message)
insight::format_error(
paste("`check_model()` returned following error:", cleaned_string),
paste0("\nIf the error message does not help identifying your problem, another reason why `check_model()` failed might be that models of class `", class(x)[1], "` are not yet supported.") # nolint
)
}

# try to find sensible default for "type" argument
Expand Down
21 changes: 19 additions & 2 deletions R/check_outliers.R
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#' 'Details'). If a numeric value is given, it will be used as the threshold
#' for any of the method run.
#' @param ID Optional, to report an ID column along with the row number.
#' @param verbose Toggle warnings.
#' @param ... When `method = "ics"`, further arguments in `...` are passed
#' down to [ICSOutlier::ics.outlier()]. When `method = "mahalanobis"`,
#' they are passed down to [stats::mahalanobis()]. `percentage_central` can
Expand Down Expand Up @@ -348,6 +349,7 @@ check_outliers.default <- function(x,
method = c("cook", "pareto"),
threshold = NULL,
ID = NULL,
verbose = TRUE,
...) {
# Check args
if (all(method == "all")) {
Expand Down Expand Up @@ -391,8 +393,23 @@ check_outliers.default <- function(x,
# Get data
my_data <- insight::get_data(x, verbose = FALSE)

# sanity check for date, POSIXt and difftime variables
if (any(vapply(my_data, inherits, FUN.VALUE = logical(1), what = c("Date", "POSIXt", "difftime"))) && verbose) {
insight::format_alert(
paste(
"Date variables are not supported for outliers detection. These will be ignored.",
"Make sure any date variables are converted to numeric or factor {.b before} fitting the model."
)
)
}

# Remove non-numerics
my_data <- datawizard::data_select(my_data, select = is.numeric)
my_data <- datawizard::data_select(my_data, select = is.numeric, verbose = FALSE)

# check if any data left
if (is.null(my_data) || ncol(my_data) == 0) {
insight::format_error("No numeric variables found. No data to check for outliers.")
}

# Thresholds
if (is.null(threshold)) {
Expand All @@ -409,7 +426,7 @@ check_outliers.default <- function(x,
)
}

if (!missing(ID)) {
if (!missing(ID) && verbose) {
insight::format_warning(paste0("ID argument not supported for model objects of class `", class(x)[1], "`."))
}

Expand Down
3 changes: 3 additions & 0 deletions man/check_outliers.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 7 additions & 0 deletions tests/testthat/test-check_collinearity.R
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,10 @@ test_that("check_collinearity, hurdle/zi models w/o zi-formula", {
)
expect_equal(out$VIF, c(1.05772, 1.05772, 1.06587, 1.06587), tolerance = 1e-4)
})

test_that("check_collinearity, invalid data", {
skip_if(packageVersion("insight") < "0.19.8.2")
dd <- data.frame(y = as.difftime(0:5, units = "days"))
m1 <- lm(y ~ 1, data = dd)
expect_error(check_collinearity(m1), "Can't extract variance-covariance matrix")
})
7 changes: 7 additions & 0 deletions tests/testthat/test-check_model.R
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,10 @@ test_that("`check_outliers()` works if convergence issues", {
x <- check_outliers(m, verbose = FALSE)
expect_s3_class(x, "check_outliers")
})

test_that("`check_model()` for invalid models", {
skip_if(packageVersion("insight") < "0.19.8.2")
dd <- data.frame(y = as.difftime(0:5, units = "days"))
m1 <- lm(y ~ 1, data = dd)
expect_error(check_model(m1))
})
13 changes: 13 additions & 0 deletions tests/testthat/test-check_outliers.R
Original file line number Diff line number Diff line change
Expand Up @@ -329,3 +329,16 @@ test_that("cook multiple methods which", {
c("setosa", "versicolor", "virginica")
)
})


test_that("check_outliers with invald data", {
dd <- data.frame(y = as.difftime(0:5, units = "days"))
m1 <- lm(y ~ 1, data = dd)
expect_error(
expect_message(
check_outliers(m1),
regex = "Date variables are not supported"
),
regex = "No numeric variables found"
)
})

0 comments on commit 082f9e8

Please sign in to comment.