fix: strip backticks from variable names in ard_car_vif() (#336)

**What changes are proposed in this pull request?**
* `ard_car_vif()` now cleans variable names by stripping backticks
before pivoting they output to the ARD structure. This ensures the
output matches the cleaned variable names in
`gtsummary::tbl_regression()`, fixing an issue where VIF values were
populated as `NA` for variables with spaces or non-syntactic names.
(#335, @NourEdinDarwish)

Previously, `car::vif()` returned names enclosed in backticks (e.g. ``
`Marker Level` ``). Since `add_vif()` performs a `left_join` against the
cleaned names from `tbl_regression()`, the join would fail. This branch
introduces a call to `broom.helpers::.clean_backticks()` to ensure the
ARD variable names match exactly what `gtsummary` expects.

**Reference GitHub issue associated with pull request.**
Fixes #335


--------------------------------------------------------------------------------

Pre-review Checklist (if item does not apply, mark is as complete)
- [ ] **All** GitHub Action workflows pass with a ✅
- [ ] PR branch has pulled the most recent updates from master branch:
`usethis::pr_merge_main()`
- [ ] If a bug was fixed, a unit test was added.
- [ ] If a new `ard_*()` function was added, it passes the ARD
structural checks from `cards::check_ard_structure()`.
- [ ] If a new `ard_*()` function was added, `set_cli_abort_call()` has
been set.
- [ ] If a new `ard_*()` function was added and it depends on another
package (such as, `broom`), `is_pkg_installed("broom")` has been set in
the function call and the following added to the roxygen comments:
`@examplesIf do.call(asNamespace("cardx")$is_pkg_installed, list(pkg =
"broom""))`
- [ ] Code coverage is suitable for any new functions/features
(generally, 100% coverage for new code): `devtools::test_coverage()`

Reviewer Checklist (if item does not apply, mark is as complete)

- [ ] If a bug was fixed, a unit test was added.
- [ ] Code coverage is suitable for any new functions/features:
`devtools::test_coverage()`

When the branch is ready to be merged:
- [ ] Update `NEWS.md` with the changes from this pull request under the
heading "`# cardx (development version)`". If there is an issue
associated with the pull request, reference it in parentheses at the end
update (see `NEWS.md` for examples).
- [ ] **All** GitHub Action workflows pass with a ✅
- [ ] Approve Pull Request
- [ ] Merge the PR. Please use "Squash and merge" or "Rebase and merge".

---------

Co-authored-by: Daniel Sjoberg <danield.sjoberg@gmail.com>

Author: NourEdinDarwish

NEWS.md

@@ -2,6 +2,8 @@
 
 * Added fix to ensure `as_card` does not error after update to `cards`
 
+* Bug fix in `ard_car_vif()` where non-syntactic variable names (e.g. those containing spaces) were returned with backticks in the `variable` column. Since `gtsummary::tbl_regression()` stores variable names without backticks, this mismatch resulted in empty VIF columns in `gtsummary::add_vif()`. (#335, @NourEdinDarwish)
+
 # cardx 0.3.2
 
 * Swapped internal use of `dplyr::case_when()` for `dplyr::recode_values()` as the former is now deprecated. (#327)

R/ard_car_vif.R

@@ -14,14 +14,14 @@
 #' @rdname ard_car_vif
 #' @export
 #'
-#' @examplesIf do.call(asNamespace("cardx")$is_pkg_installed, list(pkg = "car"))
+#' @examplesIf do.call(asNamespace("cardx")$is_pkg_installed, list(pkg = c("car", "broom.helpers")))
 #' lm(AGE ~ ARM + SEX, data = cards::ADSL) |>
 #'   ard_car_vif()
 ard_car_vif <- function(x, ...) {
   set_cli_abort_call()
 
   # check installed packages ---------------------------------------------------
-  check_pkg_installed("car")
+  check_pkg_installed(c("car", "broom.helpers"))
 
   # check inputs ---------------------------------------------------------------
   check_not_missing(x)
@@ -67,6 +67,12 @@ ard_car_vif <- function(x, ...) {
   # Clean-up the result to fit the ard structure through pivot
   vif$result <-
     vif$result |>
+    dplyr::mutate(
+      variable = broom.helpers::.clean_backticks(
+        .data$variable,
+        variable_names = broom.helpers::model_list_variables(x, only_variable = TRUE)
+      )
+    ) |>
     tidyr::pivot_longer(
       cols = -c("variable"),
       names_to = "stat_name",

man/ard_car_vif.Rd

@@ -20,6 +20,35 @@
       1    BMIBL car_vif       VIF        VIF 1.010522       1    NULL  NULL
       2  EDUCLVL car_vif       VIF        VIF 1.010522       1    NULL  NULL
 
+# ard_car_vif() works with non-syntactic predictor names
+
+    Code
+      as.data.frame(ard_car_vif(lm(AGE ~ `BMI Baseline` + EDUCLVL, data = df)))
+    Output
+            variable context stat_name stat_label     stat fmt_fun warning error
+      1 BMI Baseline car_vif       VIF        VIF 1.010522       1    NULL  NULL
+      2      EDUCLVL car_vif       VIF        VIF 1.010522       1    NULL  NULL
+
+# ard_car_vif() works with non-syntactic interaction-only terms
+
+    Code
+      as.data.frame(ard_car_vif(lm(AGE ~ ARM + `BMI Baseline`:SEX, data = df)))
+    Output
+                variable context stat_name    stat_label     stat fmt_fun warning
+      1              ARM car_vif      GVIF          GVIF 1.047141       1    NULL
+      2              ARM car_vif        df            df        2       1    NULL
+      3              ARM car_vif     aGVIF Adjusted GVIF 1.011582       1    NULL
+      4 BMI Baseline:SEX car_vif      GVIF          GVIF 1.047141       1    NULL
+      5 BMI Baseline:SEX car_vif        df            df        2       1    NULL
+      6 BMI Baseline:SEX car_vif     aGVIF Adjusted GVIF 1.011582       1    NULL
+        error
+      1  NULL
+      2  NULL
+      3  NULL
+      4  NULL
+      5  NULL
+      6  NULL
+
 # ard_vif() issues friendly messaging for incorrect object passed in/can't get terms of model
 
     Code

tests/testthat/_snaps/ard_car_vif.md

@@ -1,4 +1,4 @@
-skip_if_pkg_not_installed("car")
+skip_if_pkg_not_installed(c("car", "broom.helpers"))
 
 test_that("ard_car_vif() works", {
   expect_snapshot(
@@ -14,6 +14,28 @@ test_that("ard_car_vif() works", {
   )
 })
 
+test_that("ard_car_vif() works with non-syntactic predictor names", {
+  df <- cards::ADSL
+  names(df)[names(df) == "BMIBL"] <- "BMI Baseline"
+
+  expect_snapshot(
+    lm(AGE ~ `BMI Baseline` + EDUCLVL, data = df) |>
+      ard_car_vif() |>
+      as.data.frame()
+  )
+})
+
+test_that("ard_car_vif() works with non-syntactic interaction-only terms", {
+  df <- cards::ADSL
+  names(df)[names(df) == "BMIBL"] <- "BMI Baseline"
+
+  expect_snapshot(
+    lm(AGE ~ ARM + `BMI Baseline`:SEX, data = df) |>
+      ard_car_vif() |>
+      as.data.frame()
+  )
+})
+
 test_that("ard_car_vif() appropriate errors are given for model with only 1 term", {
   expect_equal(
     lm(AGE ~ ARM, data = cards::ADSL) |>