From 6e9d3876650e7a2c88eae60bbfda0efb553c1499 Mon Sep 17 00:00:00 2001
From: Sean Anderson <sean@seananderson.ca>
Date: Wed, 1 Nov 2023 10:14:13 -0700
Subject: [PATCH] Check for character in SVC formula #269

---
 R/fit.R                                 |  8 ++++++
 tests/testthat/test-3-spatial-varying.R | 35 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/R/fit.R b/R/fit.R
index 4db725a76..b99aeaf38 100644
--- a/R/fit.R
+++ b/R/fit.R
@@ -772,6 +772,14 @@ sdmTMB <- function(
 
   spatial_varying_formula <- spatial_varying # save it
   if (!is.null(spatial_varying)) {
+    mf1 <- model.frame(spatial_varying, data)
+    for (i in seq_len(ncol(mf1))) {
+      if (is.character(mf1[[i]])) {
+        cli_warn(paste0("Detected '{colnames(mf1)[i]}' as a character term in the ",
+          "'spatial_varying' formula. We suggest you make this a factor if you plan ",
+          "to predict with only some factor levels. `as.factor({colnames(mf1)[i]})`."))
+      }
+    }
     z_i <- model.matrix(spatial_varying, data)
     .int <- sum(grep("(Intercept)", colnames(z_i)) > 0)
     if (length(attr(z_i, "contrasts")) && !.int && !omit_spatial_intercept) { # factors with ~ 0 or ~ -1
diff --git a/tests/testthat/test-3-spatial-varying.R b/tests/testthat/test-3-spatial-varying.R
index 0bb6c8a06..ff25a26d0 100644
--- a/tests/testthat/test-3-spatial-varying.R
+++ b/tests/testthat/test-3-spatial-varying.R
@@ -109,3 +109,38 @@ test_that("Delta model with spatially varying factor predictor and no spatiotemp
   expect_s3_class(m, "sdmTMB")
   expect_true(sum(is.na(m$sd_report$sd)) == 0L)
 })
+
+test_that("Factor handling for SVC models works #269", {
+  skip_on_cran()
+  skip_on_ci()
+  set.seed(1)
+  pcod_2011$vessel <- sample(c("A", "B"), size = nrow(pcod_2011), replace = TRUE)
+  pcod_2011$vessel <- as.factor(pcod_2011$vessel)
+  fit <- sdmTMB(present ~ vessel,
+    spatial_varying = ~ vessel,
+    spatial = "on",
+    mesh = pcod_mesh_2011,
+    data = pcod_2011
+  )
+  p1 <- predict(fit, pcod_2011)
+  p2 <- predict(fit, newdata = pcod_2011)
+  expect_equal(p1$est, p2$est)
+
+  p3 <- predict(fit, newdata = pcod_2011[pcod_2011$vessel == "A", ])
+  p4 <- p2[p2$vessel == "A", ]
+  expect_equal(p3$est, p4$est)
+})
+
+test_that("SVC throws a warning if character class #269", {
+  skip_on_cran()
+  skip_on_ci()
+  pcod_2011$vessel <- sample(c("A", "B"), size = nrow(pcod_2011), replace = TRUE)
+  expect_warning({
+    fit <- sdmTMB(present ~ vessel,
+      spatial_varying = ~ vessel,
+      spatial = "on",
+      mesh = pcod_mesh_2011,
+      data = pcod_2011
+    )
+  }, regexp = "character")
+})