Skip to content

Commit

Permalink
difficult-to-diagnose errors using "difftime" response in a linear model
Browse files Browse the repository at this point in the history
Fixes #678
  • Loading branch information
strengejacke committed Feb 4, 2024
1 parent e946088 commit 6f153be
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 8 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
19 changes: 14 additions & 5 deletions R/check_collinearity.R
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,18 @@ 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-changed-files / lint-changed-files

file=R/check_collinearity.R,line=406,col=1,[cyclocomp_linter] Reduce the cyclomatic complexity of this function from 43 to at most 40.

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)
v <- .safe(insight::get_varcov(x, component = component, verbose = FALSE))

# sanity check
if (is.null(v)) {
if (isTRUE(verbose)) {
insight::format_alert(
sprintf("Could not extract the variance-covariance matrix for the %s component of the model.", component)
)
}
return(NULL)
}

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

# any assignment found?
Expand Down Expand Up @@ -432,10 +443,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.")

Check warning on line 447 in R/check_collinearity.R

View check run for this annotation

Codecov / codecov/patch

R/check_collinearity.R#L446-L447

Added lines #L446 - L447 were not covered by tests
}

f <- insight::find_formula(x)
Expand Down
16 changes: 14 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,18 @@ 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("Date variables are not supported for outliers detection. These will be ignored.")
}

# 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 +421,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.

6 changes: 6 additions & 0 deletions tests/testthat/test-check_collinearity.R
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,9 @@ 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", {
dd <- data.frame(y = as.difftime(0:5, units = "days"))
m1 <- lm(y ~ 1, data = dd)
expect_message(expect_null(check_collinearity(m1)), "Could not extract")
})
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 6f153be

Please sign in to comment.