From 392dd60a26a6dbda04a5060c31be48dcb57fd5dd Mon Sep 17 00:00:00 2001
From: Zelos Zhu <zelos.zhu@atorusresearch.com>
Date: Tue, 27 Feb 2024 15:42:47 +0000
Subject: [PATCH] address note

---
 R/ard_regression.R               |  1 +
 R/ard_vif.R                      | 43 ++++++++++++++++----------------
 tests/testthat/_snaps/ard_vif.md | 32 +++++++++++++-----------
 3 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/R/ard_regression.R b/R/ard_regression.R
index 73a70c7c..dfd62e45 100644
--- a/R/ard_regression.R
+++ b/R/ard_regression.R
@@ -44,6 +44,7 @@ ard_regression.default <- function(x, tidy_fun = broom.helpers::tidy_with_broom_
     tidyr::pivot_longer(
       cols = -c("variable", "variable_level"),
       names_to = "stat_name",
+      # noticed this while trying to fix ard_vif() does this need renaming to "statistic" ?
       values_to = "stat"
     ) |>
     dplyr::filter(map_lgl(.data$stat, Negate(is.na))) |>
diff --git a/R/ard_vif.R b/R/ard_vif.R
index c25f934e..ac933f47 100644
--- a/R/ard_vif.R
+++ b/R/ard_vif.R
@@ -21,27 +21,19 @@ ard_vif <- function(x) {
   # check inputs ---------------------------------------------------------------
   check_not_missing(x, "model")
 
-  vif_return <- .vif_to_tibble(x)
-  dplyr::tibble(
-    vif_return$result,
-    warning = vif_return["warning"],
-    error = vif_return["error"]
-  )
-}
-
-
-# put VIF results in data frame -- borrowed from gtsummary
-.vif_to_tibble <- function(x) {
   temp <- x
   vif <- car::vif(x) |>
     cards::eval_capture_conditions()
 
-  # if VIF is returned
+  # if vif failed, set result as NULL, error will be kept through eval_capture_conditions()
   if (is.null(vif$result)) {
     vif$result <- dplyr::tibble(variable = names(temp$coefficients)[-1], VIF = list(NULL))
-  } else if (!is.matrix(vif$result)) {
+  }
+  # if VIF is returned
+  else if (!is.matrix(vif$result)) {
     vif$result <- dplyr::tibble(variable = names(vif$result), VIF = vif$result)
-  } # if Generalized VIF is returned
+  }
+  # if Generalized VIF is returned
   else if (is.matrix(vif$result)) {
     vif$result <-
       vif$result |>
@@ -54,6 +46,7 @@ ard_vif <- function(x) {
       dplyr::tibble()
   }
 
+  # Clean-up the result to fit the ard structure through pivot
   vif$result <-
     vif$result |>
     tidyr::pivot_longer(
@@ -62,17 +55,23 @@ ard_vif <- function(x) {
       values_to = "statistic"
     ) |>
     dplyr::mutate(
-      .after = "variable",
-      context = "vif"
-    ) |>
-    dplyr::mutate(
-      .after = "stat_name",
+      context = "vif",
       stat_label = ifelse(
-        stat_name == "aGVIF",
+        .data$stat_name == "aGVIF",
         "Adjusted GVIF",
-        stat_name
+        .data$stat_name
       )
     )
 
-  return(vif)
+  # Bind the results and possible warning/errors together
+  vif_return <- dplyr::tibble(
+    vif$result,
+    warning = vif["warning"],
+    error = vif["error"]
+    )
+
+  # Clean up return object
+  vif_return |>
+   cards::tidy_ard_column_order() %>%
+    {structure(., class = c("card", class(.)))}  # styler: off
 }
diff --git a/tests/testthat/_snaps/ard_vif.md b/tests/testthat/_snaps/ard_vif.md
index b79ea200..c08074f1 100644
--- a/tests/testthat/_snaps/ard_vif.md
+++ b/tests/testthat/_snaps/ard_vif.md
@@ -2,25 +2,29 @@
 
     Code
       ard_vif(lm(AGE ~ ARM + SEX, data = cards::ADSL))
+    Message
+      {cards} data frame: 6 x 7
     Output
-      # A tibble: 6 x 7
-        variable context stat_name stat_label    statistic warning      error       
-        <chr>    <chr>   <chr>     <chr>             <dbl> <named list> <named list>
-      1 ARM      vif     GVIF      GVIF               1.02 <NULL>       <NULL>      
-      2 ARM      vif     df        df                 2    <NULL>       <NULL>      
-      3 ARM      vif     aGVIF     Adjusted GVIF      1.00 <NULL>       <NULL>      
-      4 SEX      vif     GVIF      GVIF               1.02 <NULL>       <NULL>      
-      5 SEX      vif     df        df                 1    <NULL>       <NULL>      
-      6 SEX      vif     aGVIF     Adjusted GVIF      1.01 <NULL>       <NULL>      
+        variable context stat_name stat_label statistic
+      1      ARM     vif      GVIF       GVIF  1.015675
+      2      ARM     vif        df         df  2.000000
+      3      ARM     vif     aGVIF  Adjusted…  1.003896
+      4      SEX     vif      GVIF       GVIF  1.015675
+      5      SEX     vif        df         df  1.000000
+      6      SEX     vif     aGVIF  Adjusted…  1.007807
+    Message
+      i 2 more variables: warning, error
 
 # ard_vif() appropriate errors are given for model with only 1 term
 
     Code
       ard_vif(lm(AGE ~ ARM, data = cards::ADSL))
+    Message
+      {cards} data frame: 2 x 7
     Output
-      # A tibble: 2 x 7
-        variable                context stat_name stat_label statistic warning   error
-        <chr>                   <chr>   <chr>     <chr>      <list>    <named l> <nam>
-      1 ARMXanomeline High Dose vif     VIF       VIF        <NULL>    <NULL>    <chr>
-      2 ARMXanomeline Low Dose  vif     VIF       VIF        <NULL>    <NULL>    <chr>
+                       variable context stat_name stat_label     error statistic
+      1 ARMXanomeline High Dose     vif       VIF        VIF model co…      NULL
+      2  ARMXanomeline Low Dose     vif       VIF        VIF model co…      NULL
+    Message
+      i 1 more variable: warning