From 78760beb12e0fc4b163ad52549d0c6bbbfc642c3 Mon Sep 17 00:00:00 2001
From: Daniel <mail@danielluedecke.de>
Date: Thu, 7 Nov 2024 08:41:27 +0100
Subject: [PATCH 1/6] `fixest`: Incorrect standard errors and error Fixes #1039

---
 DESCRIPTION                                   |  3 ++-
 NEWS.md                                       |  3 +++
 R/methods_fixest.R                            | 10 ++++++++--
 tests/testthat/test-model_parameters.fixest.R | 17 +++++++++++++++++
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/DESCRIPTION b/DESCRIPTION
index c74c1383c..1d8394519 100644
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -1,7 +1,7 @@
 Type: Package
 Package: parameters
 Title: Processing of Model Parameters
-Version: 0.23.0.6
+Version: 0.23.0.7
 Authors@R:
     c(person(given = "Daniel",
              family = "Lüdecke",
@@ -113,6 +113,7 @@ Suggests:
     coxme,
     cplm,
     dbscan,
+    did,
     distributional,
     domir (>= 0.2.0),
     drc,
diff --git a/NEWS.md b/NEWS.md
index a97e5c76f..e232e619f 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -10,6 +10,9 @@
 * Fixed bug when extracting 'pretty labels' for model parameters, which could
   fail when predictors were character vectors.
 
+* Fixed bug with inaccurate standard errors for models from package *fixest*
+  that used the `sunab()` function in the formula.
+
 # parameters 0.23.0
 
 ## Breaking Changes
diff --git a/R/methods_fixest.R b/R/methods_fixest.R
index 7bfc9a945..fa331dbf9 100644
--- a/R/methods_fixest.R
+++ b/R/methods_fixest.R
@@ -73,8 +73,14 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
   params <- insight::get_parameters(model)
 
   if (is.null(vcov)) {
-    stats <- summary(model)
-    SE <- as.vector(stats$se)
+    # most reliable is from "coeftable" element
+    stats <- model$coeftable
+    SE <- .safe(as.vector(stats[, "Std. Error"]))
+    # if this fails extract from summary
+    if (is.null(SE)) {
+      stats <- summary(model)
+      SE <- as.vector(stats$se)
+    }
   } else {
     # we don't want to wrap this in a tryCatch because the `fixest` error is
     # informative when `vcov` is wrong.
diff --git a/tests/testthat/test-model_parameters.fixest.R b/tests/testthat/test-model_parameters.fixest.R
index 567ba491f..16dc84313 100644
--- a/tests/testthat/test-model_parameters.fixest.R
+++ b/tests/testthat/test-model_parameters.fixest.R
@@ -115,3 +115,20 @@ test_that("robust standard errors", {
   expect_error(parameters(mod, vcov = "hetero"), NA)
   expect_error(parameters(mod, vcov = "iid"), NA)
 })
+
+
+test_that("standard errors, Sun and Abraham", {
+  data(mpdta, package = "did")
+  m <- fixest::feols(
+    lemp ~ sunab(first.treat, year, ref.p = -1:-4, att = TRUE) | countyreal + year,
+    data = mpdta,
+    cluster = ~countyreal
+  )
+  out <- model_parameters(m)
+  expect_equal(out$SE, m$coeftable[, "Std. Error"], tolerance = 1e-4, ignore_attr = TRUE)
+
+  data(base_stagg, package = "fixest")
+  m <- fixest::feols(y ~ x1 + sunab(year_treated, year) | id + year, base_stagg)
+  out <- model_parameters(m)
+  expect_equal(out$SE, m$coeftable[, "Std. Error"], tolerance = 1e-4, ignore_attr = TRUE)
+})

From 30bccb891cac2d79db2578a811b823668b6174cf Mon Sep 17 00:00:00 2001
From: Daniel <mail@danielluedecke.de>
Date: Thu, 7 Nov 2024 11:41:17 +0100
Subject: [PATCH 2/6] fix

---
 R/methods_fixest.R | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/R/methods_fixest.R b/R/methods_fixest.R
index fa331dbf9..83cd8c444 100644
--- a/R/methods_fixest.R
+++ b/R/methods_fixest.R
@@ -73,13 +73,16 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
   params <- insight::get_parameters(model)
 
   if (is.null(vcov)) {
-    # most reliable is from "coeftable" element
+    # get standard errors from summary
+    stats <- summary(model)
+    SE <- as.vector(stats$se)
+    # sometimes, the correct SEs are in the `coeftable`. In this case,
+    # SE from summary have different length
     stats <- model$coeftable
-    SE <- .safe(as.vector(stats[, "Std. Error"]))
+    SE_ct <- .safe(as.vector(stats[, "Std. Error"]))
     # if this fails extract from summary
-    if (is.null(SE)) {
-      stats <- summary(model)
-      SE <- as.vector(stats$se)
+    if (!is.null(SE_ct) && length(SE_ct) < length(SE)) {
+      SE <- SE_ct
     }
   } else {
     # we don't want to wrap this in a tryCatch because the `fixest` error is

From 1ffacf1909fb1abb8ebe31aa94c6011bf83f7543 Mon Sep 17 00:00:00 2001
From: Daniel <mail@danielluedecke.de>
Date: Thu, 7 Nov 2024 12:02:43 +0100
Subject: [PATCH 3/6] fix

---
 tests/testthat/test-model_parameters.fixest.R | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/testthat/test-model_parameters.fixest.R b/tests/testthat/test-model_parameters.fixest.R
index 16dc84313..50cf3ea5b 100644
--- a/tests/testthat/test-model_parameters.fixest.R
+++ b/tests/testthat/test-model_parameters.fixest.R
@@ -118,6 +118,7 @@ test_that("robust standard errors", {
 
 
 test_that("standard errors, Sun and Abraham", {
+  skip_if_not_installed("did")
   data(mpdta, package = "did")
   m <- fixest::feols(
     lemp ~ sunab(first.treat, year, ref.p = -1:-4, att = TRUE) | countyreal + year,

From 57f19d59da77911ef59c1ce503c6799f2c613271 Mon Sep 17 00:00:00 2001
From: Daniel <mail@danielluedecke.de>
Date: Thu, 7 Nov 2024 12:07:37 +0100
Subject: [PATCH 4/6] fix

---
 R/methods_fixest.R | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/R/methods_fixest.R b/R/methods_fixest.R
index 83cd8c444..8f02249b1 100644
--- a/R/methods_fixest.R
+++ b/R/methods_fixest.R
@@ -78,11 +78,13 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
     SE <- as.vector(stats$se)
     # sometimes, the correct SEs are in the `coeftable`. In this case,
     # SE from summary have different length
-    stats <- model$coeftable
-    SE_ct <- .safe(as.vector(stats[, "Std. Error"]))
-    # if this fails extract from summary
-    if (!is.null(SE_ct) && length(SE_ct) < length(SE)) {
-      SE <- SE_ct
+    if (length(SE) != nrow(params)) {
+      SE <- NULL
+      stats <- model$coeftable
+      SE_ct <- .safe(as.vector(stats[, "Std. Error"]))
+      if (!is.null(SE_ct) && length(SE_ct) == nrow(params)) {
+        SE <- SE_ct
+      }
     }
   } else {
     # we don't want to wrap this in a tryCatch because the `fixest` error is
@@ -91,6 +93,11 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
     SE <- sqrt(diag(V))
   }
 
+  if (is.null(SE)) {
+    SE <- NA
+    insight::format_alert("Could not extract standard errors. Please file an issue at https://github.com/easystats/parameters/issues/")
+  }
+
   .data_frame(
     Parameter = params$Parameter,
     SE = SE

From f709ff3ab94a2030a15aaa49037f5b2f3aa509fe Mon Sep 17 00:00:00 2001
From: Daniel <mail@danielluedecke.de>
Date: Thu, 7 Nov 2024 12:08:22 +0100
Subject: [PATCH 5/6] minor

---
 R/methods_fixest.R | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/R/methods_fixest.R b/R/methods_fixest.R
index 8f02249b1..555bd2467 100644
--- a/R/methods_fixest.R
+++ b/R/methods_fixest.R
@@ -80,8 +80,7 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
     # SE from summary have different length
     if (length(SE) != nrow(params)) {
       SE <- NULL
-      stats <- model$coeftable
-      SE_ct <- .safe(as.vector(stats[, "Std. Error"]))
+      SE_ct <- .safe(as.vector(model$coeftable[, "Std. Error"]))
       if (!is.null(SE_ct) && length(SE_ct) == nrow(params)) {
         SE <- SE_ct
       }

From b47d9e42ec4cb6865539bafccbc850316708e0b7 Mon Sep 17 00:00:00 2001
From: Daniel <mail@danielluedecke.de>
Date: Thu, 7 Nov 2024 16:29:50 +0100
Subject: [PATCH 6/6] fix

---
 R/methods_fixest.R | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/R/methods_fixest.R b/R/methods_fixest.R
index 555bd2467..ca058dcb4 100644
--- a/R/methods_fixest.R
+++ b/R/methods_fixest.R
@@ -74,17 +74,9 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
 
   if (is.null(vcov)) {
     # get standard errors from summary
+    # see https://github.com/easystats/parameters/issues/1039
     stats <- summary(model)
-    SE <- as.vector(stats$se)
-    # sometimes, the correct SEs are in the `coeftable`. In this case,
-    # SE from summary have different length
-    if (length(SE) != nrow(params)) {
-      SE <- NULL
-      SE_ct <- .safe(as.vector(model$coeftable[, "Std. Error"]))
-      if (!is.null(SE_ct) && length(SE_ct) == nrow(params)) {
-        SE <- SE_ct
-      }
-    }
+    SE <- as.vector(stats$coeftable[, "Std. Error"])
   } else {
     # we don't want to wrap this in a tryCatch because the `fixest` error is
     # informative when `vcov` is wrong.
@@ -92,11 +84,6 @@ standard_error.fixest <- function(model, vcov = NULL, vcov_args = NULL, ...) {
     SE <- sqrt(diag(V))
   }
 
-  if (is.null(SE)) {
-    SE <- NA
-    insight::format_alert("Could not extract standard errors. Please file an issue at https://github.com/easystats/parameters/issues/")
-  }
-
   .data_frame(
     Parameter = params$Parameter,
     SE = SE