Merge branch 'issue-301': closes #301

gavinsimpson · Jul 27, 2024 · ae4a049 · ae4a049
2 parents ffed12e + 665ded2
commit ae4a049
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 5 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: gratia
-Version: 0.9.2.9004
-Date: 2024-07-17
+Version: 0.9.2.9005
+Date: 2024-07-27
 Title: Graceful 'ggplot'-Based Graphics and Other Functions for GAMs Fitted Using 'mgcv'
 Authors@R: c(person(given = "Gavin L.", family = "Simpson",
                     email = "ucfagls@gmail.com",

diff --git a/NEWS.md b/NEWS.md
@@ -18,6 +18,9 @@
 * `draw()` was not plotting cyclic P spline smooths. Reported by @Zuckerbrot
   #297
 
+* `derivatives()` would fail for `"fs"` smooths with other parametric effects in
+  the model. Reported by @mahonmb #301
+
 # gratia 0.9.2
 
 ## Breaking changes

diff --git a/R/derivatives.R b/R/derivatives.R
@@ -556,7 +556,8 @@
   ## because model.frame() on a gamm() contains extraneous variables, related
   ## to the mixed model form for lme()
   all_m_vars <- m_vars <- model_vars(model)
-  want <- str_detect(smooth_vars, m_vars)
+  # want <- str_detect(smooth_vars, m_vars)
+  want <- m_vars %in% smooth_vars
   m_vars <- m_vars[want]
 
   # now get rid of all but the focal variable *if set* in m_vars

diff --git a/tests/testthat/test-derivatives.R b/tests/testthat/test-derivatives.R
@@ -441,7 +441,7 @@ test_that("derivatives() works for factor by smooths issue 47", {
   expect_doppelganger("draw issue 47 derivatives for gamm factor by", plt4)
 })
 
-test_that("derivatives() works for fs smooths issue 57", {
+test_that("derivatives() works for fs smooths issue 57 and 301", {
   skip_on_cran()
   logistic.growth <- function(t, y0, k, r) {
     return(k * (y0 / (y0 + (k - y0) * exp(-r * t))))
@@ -472,9 +472,15 @@ test_that("derivatives() works for fs smooths issue 57", {
   expect_s3_class(fd, "tbl_df")
   expect_named(fd, c(deriv_nms, "t", "unit"))
   plt <- draw(fd) # FIXME: need to update draw(d) so it works with fs smooths
-  
+
   skip_on_ci() # testing without as moved to mac os x
   expect_doppelganger("draw issue 57 derivatives for factor by", plt)
+
+  # test variant in #301
+  logistic_growth_df$linearterm = runif(16)
+  m <- gam(y_obs ~ s(t, unit, k = 5, bs = "fs", m = 2) + linearterm,
+    data = logistic_growth_df, method = "REML")
+  expect_silent(m <- derivatives(m))
 })
 
 ## tests for by variables & simultaneous intervals #102