550: Clarify error message when factor is required (#553)

* Clarify error message when factor is required

* use mathjax for math rendering in `pkgdown` to fix rendering issue

* remove duplicate assertion, update test

Author: danielinteractive

R/tmb.R

@@ -288,7 +288,13 @@ h_mmrm_tmb_data <- function(
     n_visits <- max(subject_n_visits)
   } else {
     assert(identical(ncol(coordinates), 1L))
-    assert_factor(coordinates[[1L]])
+    if (!test_factor(coordinates[[1L]])) {
+      stop(
+        "Time point variable '",
+        formula_parts$visit_var,
+        "' must be a factor."
+      )
+    }
     coordinates_matrix <- as.matrix(as.integer(coordinates[[1L]]) - 1, ncol = 1)
     n_visits <- nlevels(coordinates[[1L]])
     assert_true(all(subject_n_visits <= n_visits))
@@ -618,14 +624,6 @@ fit_mmrm <- function(
     covariance <- h_reconcile_cov_struct(formula, covariance)
     formula_parts <- h_mmrm_tmb_formula_parts(formula, covariance)
 
-    if (
-      !formula_parts$is_spatial && !is.factor(data[[formula_parts$visit_var]])
-    ) {
-      stop(
-        "Time variable must be a factor for non-spatial covariance structures"
-      )
-    }
-
     assert_class(control, "mmrm_control")
     assert_list(control$optimizers, min.len = 1)
     assert_numeric(weights, any.missing = FALSE)

_pkgdown.yml

@@ -5,6 +5,7 @@ template:
   bootstrap: 5
   params:
     ganalytics: UA-125641273-1
+  math-rendering: "mathjax"
 
 navbar:
   structure:

tests/testthat/test-fit.R

@@ -546,6 +546,16 @@ test_that("divergent optimizer will not be marked as success", {
   )
 })
 
+test_that("mmrm provides readable error message when visit variable is not a factor", {
+  fev <- fev_data
+  fev$AVISIT <- as.character(fev$AVISIT)
+  expect_error(
+    mmrm(FEV1 ~ AVISIT + us(AVISIT | USUBJID), data = fev),
+    "Time point variable 'AVISIT' must be a factor.",
+    fixed = TRUE
+  )
+})
+
 test_that("mmrm will use the provided contrast", {
   fev2 <- fev_data
   contrasts(fev2$ARMCD) <- contr.poly(n = 2)

tests/testthat/test-tmb.R

@@ -2631,7 +2631,8 @@ test_that("fit_mmrm throws informative error for non-spatial cov with non-factor
       data = tmp_data,
       weights = rep(1, nrow(tmp_data))
     ),
-    "Time variable must be a factor for non-spatial covariance structures"
+    "Time point variable 'AVISIT' must be a factor",
+    fixed = TRUE
   )
 })