Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parameters() fails on binomial mgcv::gam #1052

Merged
merged 7 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Type: Package
Package: parameters
Title: Processing of Model Parameters
Version: 0.24.0.2
Version: 0.24.0.3
Authors@R:
c(person(given = "Daniel",
family = "Lüdecke",
Expand Down Expand Up @@ -224,4 +224,4 @@ Config/testthat/edition: 3
Config/testthat/parallel: true
Config/Needs/website: easystats/easystatstemplate
Config/rcmdcheck/ignore-inconsequential-notes: true
Remotes: easystats/datawizard
Remotes: easystats/datawizard, easystats/performance
9 changes: 9 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
# parameters 0.24.1

## Bug fixes

* Fixed issue when printing `model_parameters()` with models from `mgcv::gam()`.

* Fixed issues due to breaking changes in the latest release of the *datawizard*
package.

# parameters 0.24.0

## Breaking Changes
Expand Down
2 changes: 1 addition & 1 deletion R/dominance_analysis.R
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@
#'
#' dominance_analysis(model_wt, quote_args = "weights")
#' @export
dominance_analysis <- function(model, sets = NULL, all = NULL,

Check warning on line 135 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=135,col=1,[cyclocomp_linter] Reduce the cyclomatic complexity of this function from 53 to at most 40.
conditional = TRUE, complete = TRUE,
quote_args = NULL, contrasts = model$contrasts,
...) {
Expand All @@ -156,7 +156,7 @@
}

model_info <- insight::model_info(model)
if (any(unlist(model_info[c("is_bayesian", "is_mixed", "is_gam", "is_multivariate", "is_zero_inflated", "is_hurdle")]))) {

Check warning on line 159 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=159,col=121,[line_length_linter] Lines should not be more than 120 characters. This line is 124 characters.
insight::format_error(
paste0("`dominance_analysis()` does not yet support models of class `", class(model)[[1]], "`."),
"You may be able to dominance analyze this model using the {.pkg domir} package."
Expand Down Expand Up @@ -228,7 +228,7 @@
reg <- as.list(insight::get_call(model))[[1]]

# Process sets ----
if (!is.null(sets)) {

Check warning on line 231 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=231,col=7,[if_not_else_linter] Prefer `if (A) x else y` to the less-readable `if (!A) y else x` in a simple if/else statement.
# gather predictors from each set
sets_processed <- lapply(sets, function(x) attr(stats::terms(x), "term.labels"))

Expand All @@ -250,7 +250,7 @@
# apply names to sets
set_names <- names(sets)

missing_set_names <- which(set_names == "")

Check warning on line 253 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=253,col=32,[nzchar_linter] Use !nzchar(x) instead of x == "". Note that unlike nzchar(), EQ coerces to character, so you'll have to use as.character() if x is a factor. Whenever missing data is possible, please take care to use nzchar(., keepNA = TRUE); nzchar(NA) is TRUE by default.

if (length(missing_set_names) > 0) {
set_names[missing_set_names] <- paste0("set", missing_set_names)
Expand All @@ -277,7 +277,7 @@
}

# Process all ----
if (!is.null(all)) {

Check warning on line 280 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=280,col=7,[if_not_else_linter] Prefer `if (A) x else y` to the less-readable `if (!A) y else x` in a simple if/else statement.
# gather predictors in all
all_processed <- attr(stats::terms(all), "term.labels")

Expand Down Expand Up @@ -331,9 +331,9 @@
if (length(ivs) == 0) ivs <- "1"
fml <- stats::reformulate(ivs, response = dv, intercept = insight::has_intercept(model))

data <- insight::get_data(model, verbose = FALSE)

Check warning on line 334 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=334,col=3,[object_overwrite_linter] 'data' is an exported object from package 'utils'. Avoid re-using such symbols.

args <- as.list(insight::get_call(model), collapse = "") # extract all arguments from call

Check warning on line 336 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=336,col=3,[object_overwrite_linter] 'args' is an exported object from package 'base'. Avoid re-using such symbols.

loc <- which(!(names(args) %in% c("formula", "data"))) # find formula and data arguments

Expand All @@ -342,8 +342,8 @@
insight::format_error("Model submitted does not have a formula and `data` argument.")
}

args <- args[loc] # remove formula and data arguments

Check warning on line 345 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=345,col=3,[object_overwrite_linter] 'args' is an exported object from package 'base'. Avoid re-using such symbols.
args <- args[-1] # remove function name

Check warning on line 346 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=346,col=3,[object_overwrite_linter] 'args' is an exported object from package 'base'. Avoid re-using such symbols.

# quote arguments for domin
for (arg in quote_args) {
Expand Down Expand Up @@ -443,7 +443,7 @@
# Apply set names
if (!is.null(sets)) {
for (set in seq_along(sets)) {
set_name <- if (!is.null(names(sets)[[set]])) {

Check warning on line 446 in R/dominance_analysis.R

View workflow job for this annotation

GitHub Actions / lint-changed-files / lint-changed-files

file=R/dominance_analysis.R,line=446,col=23,[if_not_else_linter] Prefer `if (A) x else y` to the less-readable `if (!A) y else x` in a simple if/else statement.
names(sets)[[set]]
} else {
paste0("set", set)
Expand Down Expand Up @@ -571,7 +571,7 @@
}

da_df_res <- datawizard::data_rename(
da_df_res,
da_df_res,
replacement = c(
"Parameter", "General_Dominance",
"Percent", "Ranks", "Subset"
Expand Down
7 changes: 6 additions & 1 deletion R/format.R
Original file line number Diff line number Diff line change
Expand Up @@ -887,9 +887,14 @@ format.parameters_sem <- function(x,
logit_model <- isTRUE(.additional_arguments(x, "logit_link", FALSE)) ||
isTRUE(attributes(x)$coefficient_name %in% c("Log-Odds", "Odds Ratio"))

# remove NA and infinite values from spurios coefficients
if (!is.null(spurious_coefficients)) {
spurious_coefficients <- spurious_coefficients[!is.na(spurious_coefficients) & !is.infinite(spurious_coefficients)] # nolint
}

# check for complete separation coefficients or possible issues with
# too few data points
if (!is.null(spurious_coefficients) && logit_model) {
if (!is.null(spurious_coefficients) && length(spurious_coefficients) && logit_model) {
if (any(spurious_coefficients > 50)) {
msg <- c(msg, "Some coefficients are very large, which may indicate issues with complete separation.") # nolint
} else if (any(spurious_coefficients > 15)) {
Expand Down
21 changes: 21 additions & 0 deletions tests/testthat/_snaps/printing.md
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,24 @@
Uncertainty intervals (equal-tailed) and p-values (two-tailed) computed
using a Wald t-distribution approximation.

# no fail for mgcv-binomial

Code
print(out)
Output
# Fixed Effects

Parameter | Log-Odds | SE | 95% CI | z | df | p
---------------------------------------------------------------------
(Intercept) | -0.20 | 0.50 | [-1.18, 0.79] | -0.39 | 29.98 | 0.695

# Smooth Terms

Parameter | z | df | p
---------------------------------------
Smooth term (mpg) | 7.24 | 1.02 | 0.007
Message

The model has a log- or logit-link. Consider using `exponentiate =
TRUE` to interpret coefficients as ratios.

10 changes: 10 additions & 0 deletions tests/testthat/test-printing.R
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,13 @@ withr::with_options(
expect_snapshot(print(out))
})
)

withr::with_options(
list(parameters_warning_exponentiate = TRUE),
test_that("no fail for mgcv-binomial", {
skip_if_not_installed("mgcv")
m <- mgcv::gam(vs ~ s(mpg), data = mtcars, family = "binomial")
out <- model_parameters(m)
expect_snapshot(print(out))
})
)
Loading