From fd11fb6d2158967e4718c4302b40b29d6bad49cf Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 9 Sep 2023 11:14:49 +0200 Subject: [PATCH 1/6] Wrong DFs in `model_parameters.fixest` Fixes #892 --- NAMESPACE | 2 ++ R/methods_fixest.R | 64 ++++++++++++++++++++++++++++++++++++++++++++++ man/p_function.Rd | 2 +- 3 files changed, 67 insertions(+), 1 deletion(-) diff --git a/NAMESPACE b/NAMESPACE index a4ef304d9..ca6fa4760 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -274,7 +274,9 @@ S3method(model_parameters,emm_list) S3method(model_parameters,epi.2by2) S3method(model_parameters,fa) S3method(model_parameters,fa.ci) +S3method(model_parameters,feglm) S3method(model_parameters,fitdistr) +S3method(model_parameters,fixest) S3method(model_parameters,fixest_multi) S3method(model_parameters,flac) S3method(model_parameters,flic) diff --git a/R/methods_fixest.R b/R/methods_fixest.R index 7add00757..0cd3e389e 100644 --- a/R/methods_fixest.R +++ b/R/methods_fixest.R @@ -1,5 +1,66 @@ # .fixest ----------------------- +#' @export +model_parameters.fixest <- function(model, + ci = 0.95, + ci_method = NULL, + bootstrap = FALSE, + iterations = 1000, + standardize = NULL, + exponentiate = FALSE, + p_adjust = NULL, + summary = getOption("parameters_summary", FALSE), + keep = NULL, + drop = NULL, + verbose = TRUE, + vcov = NULL, + vcov_args = NULL, + ...) { + # default ci-method, based on statistic + if (is.null(ci_method)) { + if (identical(insight::find_statistic(model), "t-statistic")) { + ci_method <- "residual" + } else { + ci_method <- "wald" + } + } + + # extract model parameters table, as data frame + out <- tryCatch( + { + .model_parameters_generic( + model = model, + ci = ci, + ci_method = ci_method, + bootstrap = bootstrap, + iterations = iterations, + merge_by = "Parameter", + standardize = standardize, + exponentiate = exponentiate, + p_adjust = p_adjust, + summary = summary, + keep_parameters = keep, + drop_parameters = drop, + vcov = vcov, + vcov_args = vcov_args, + verbose = verbose, + ... + ) + }, + error = function(e) { + NULL + } + ) + + if (is.null(out)) { + insight::format_error("Something went wrong... :-/") + } + + attr(out, "object_name") <- insight::safe_deparse_symbol(substitute(model)) + out +} + + #' @export standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) { params <- insight::get_parameters(model) @@ -45,6 +106,9 @@ degrees_of_freedom.fixest <- function(model, method = "wald", ...) { # .feglm ----------------------- +#' @export +model_parameters.feglm <- model_parameters.fixest + #' @export standard_error.feglm <- function(model, ...) { stats <- stats::coef(summary(model)) diff --git a/man/p_function.Rd b/man/p_function.Rd index 6748d2c37..c84e61246 100644 --- a/man/p_function.Rd +++ b/man/p_function.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/p_function.r +% Please edit documentation in R/p_function.R \name{p_function} \alias{p_function} \alias{consonance_function} From fa44f61f05830ea89e4251ac90bc980b611aab9b Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 9 Sep 2023 11:15:50 +0200 Subject: [PATCH 2/6] desc, version --- DESCRIPTION | 2 +- NEWS.md | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 8c081ef0f..f6155e000 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: parameters Title: Processing of Model Parameters -Version: 0.21.1.2 +Version: 0.21.1.3 Authors@R: c(person(given = "Daniel", family = "Lüdecke", diff --git a/NEWS.md b/NEWS.md index 2438b90b4..c2e19793b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,9 @@ ## Bug fixes +* Fixed issue with wrong calculation of test-statistic and p-values in + `model_parameters()` for `fixest` models. + * Minor fixes for `dominance_analysis()`. # parameters 0.21.1 From a131fcab8d86403dde5e5aa585780a7da943384c Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 9 Sep 2023 13:21:53 +0200 Subject: [PATCH 3/6] Wrong DFs in `model_parameters.fixest` Fixes #892 --- R/methods_fixest.R | 12 +++++++++--- tests/testthat/test-model_parameters.fixest.R | 2 +- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/R/methods_fixest.R b/R/methods_fixest.R index 0cd3e389e..b9c3b855b 100644 --- a/R/methods_fixest.R +++ b/R/methods_fixest.R @@ -18,10 +18,10 @@ model_parameters.fixest <- function(model, ...) { # default ci-method, based on statistic if (is.null(ci_method)) { - if (identical(insight::find_statistic(model), "t-statistic")) { + if (identical(insight::find_statistic(model), "t-statistic") && identical(model$method, "feols")) { ci_method <- "residual" } else { - ci_method <- "wald" + ci_method <- "normal" } } @@ -92,8 +92,14 @@ degrees_of_freedom.fixest <- function(model, method = "wald", ...) { } method <- match.arg( tolower(method), - choices = c("wald", "residual") + choices = c("wald", "residual", "normal") ) + + # we may have Inf DF, too + if (method == "normal") { + return(Inf) + } + method <- switch(method, "wald" = "t", "residual" = "resid" diff --git a/tests/testthat/test-model_parameters.fixest.R b/tests/testthat/test-model_parameters.fixest.R index 7efb791d5..0ba9f67f2 100644 --- a/tests/testthat/test-model_parameters.fixest.R +++ b/tests/testthat/test-model_parameters.fixest.R @@ -13,7 +13,7 @@ test_that("model_parameters.fixest", { ) params <- model_parameters(m, verbose = FALSE) - expect_equal(c(nrow(params), ncol(params)), c(2, 9)) + expect_identical(c(nrow(params), ncol(params)), c(2L, 9L)) expect_equal(params$p, as.vector(fixest::pvalue(m)), tolerance = 1e-3) expect_equal(params$df_error[1], as.vector(fixest::degrees_freedom(m, type = "t")), tolerance = 1e-3) expect_equal(params$Coefficient, as.vector(coef(m)), tolerance = 1e-3) From 16f075759f8aa4f1fc90550435ed581ee2a49ee2 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 9 Sep 2023 14:52:10 +0200 Subject: [PATCH 4/6] add test --- tests/testthat/test-model_parameters.fixest.R | 27 ++++++++++++++++--- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-model_parameters.fixest.R b/tests/testthat/test-model_parameters.fixest.R index 0ba9f67f2..d6d2a5318 100644 --- a/tests/testthat/test-model_parameters.fixest.R +++ b/tests/testthat/test-model_parameters.fixest.R @@ -14,17 +14,36 @@ test_that("model_parameters.fixest", { params <- model_parameters(m, verbose = FALSE) expect_identical(c(nrow(params), ncol(params)), c(2L, 9L)) - expect_equal(params$p, as.vector(fixest::pvalue(m)), tolerance = 1e-3) - expect_equal(params$df_error[1], as.vector(fixest::degrees_freedom(m, type = "t")), tolerance = 1e-3) - expect_equal(params$Coefficient, as.vector(coef(m)), tolerance = 1e-3) + expect_equal(params$p, as.vector(fixest::pvalue(m)), tolerance = 1e-4) + expect_equal(params$df_error[1], as.vector(fixest::degrees_freedom(m, type = "t")), tolerance = 1e-4) + expect_equal(params$Coefficient, as.vector(coef(m)), tolerance = 1e-4) # currently, a bug for fixest 10.4 on R >= 4.3 - skip_if_not(getRversion() < "4.2.0") + # skip_if_not(getRversion() < "4.2.0") expect_snapshot( model_parameters(m, summary = TRUE, verbose = FALSE) ) }) + +test_that("model_parameters.fixest", { + skip_on_cran() + skip_if_not_installed("fixest") + skip_if_not_installed("carData") + + data(Greene, package = "carData") + d <- Greene + d$dv <- as.numeric(Greene$decision == "yes") + + mod1 <- fixest::feglm(dv ~ language | judge, + data = d, + cluster = "judge", family = "logit" + ) + out1 <- model_parameters(mod1) + expect_equal(out1$p, fixest::pvalue(mod1), tolerance = 1e-4) +}) + + test_that("robust standard errors", { skip_if_not_installed("fixest") mod <- fixest::feols(mpg ~ hp + am | cyl, data = mtcars) From 5a6e3769be5921c66f763d36e3ddb90a9653a515 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 9 Sep 2023 14:53:46 +0200 Subject: [PATCH 5/6] fix test --- tests/testthat/test-model_parameters.fixest.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-model_parameters.fixest.R b/tests/testthat/test-model_parameters.fixest.R index d6d2a5318..8bdc3e70e 100644 --- a/tests/testthat/test-model_parameters.fixest.R +++ b/tests/testthat/test-model_parameters.fixest.R @@ -40,7 +40,8 @@ test_that("model_parameters.fixest", { cluster = "judge", family = "logit" ) out1 <- model_parameters(mod1) - expect_equal(out1$p, fixest::pvalue(mod1), tolerance = 1e-4) + expect_equal(out1$p, as.vector(fixest::pvalue(mod1)), tolerance = 1e-4, ignore_attr = TRUE) + expect_equal(out1$SE, as.vector(sqrt(diag(vcov(mod1)))), tolerance = 1e-4, ignore_attr = TRUE) }) From 94dc3eb710aa661bf0dfcc867fe02f6fad817569 Mon Sep 17 00:00:00 2001 From: Daniel Date: Sat, 9 Sep 2023 15:52:59 +0200 Subject: [PATCH 6/6] fix --- R/methods_fixest.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/methods_fixest.R b/R/methods_fixest.R index b9c3b855b..95330f4a1 100644 --- a/R/methods_fixest.R +++ b/R/methods_fixest.R @@ -19,7 +19,7 @@ model_parameters.fixest <- function(model, # default ci-method, based on statistic if (is.null(ci_method)) { if (identical(insight::find_statistic(model), "t-statistic") && identical(model$method, "feols")) { - ci_method <- "residual" + ci_method <- "wald" } else { ci_method <- "normal" }