Skip to content

Commit 95900bd

Browse files
ehrlingerclaude
andauthored
fix: add default S3 methods to varPro-family wrappers (gg_isopro, gg_beta_varpro, gg_ivarpro) (#136)
* fix(gg_isopro): add default S3 method, remove dead inner guard gg_isopro had no default method: the friendly "expects an isopro object" stop() lived inside gg_isopro.isopro, which is only reachable when the object already inherits 'isopro', so it was dead code. A wrong-class input got R's stock "no applicable method for 'gg_isopro'" error instead. Add gg_isopro.default emitting the clear class error (matching the gg_beta_uvarpro / gg_sdependent pattern) and drop the unreachable inner check. Adds a dispatch-error test (needs no varPro fit, runs on CRAN). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_beta_varpro,gg_ivarpro): add default S3 methods, remove dead guards Same fix as gg_isopro, extended to its two varPro-family siblings so the whole family is consistent with gg_beta_uvarpro / gg_sdependent. Each had an unreachable `if (!inherits(object, "varpro")) stop(...)` inside its .varpro method; a wrong-class input got R's generic "no applicable method" error. Add gg_beta_varpro.default and gg_ivarpro.default emitting the clear class error, drop the dead inner guards, and add fit-free dispatch-error tests (run on CRAN). The 6 classic wrappers (gg_error/gg_vimp/gg_variable/ gg_rfsrc/gg_roc/gg_brier) still lack .default but have no dead guards and are deferred to a post-release consistency pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(gg_beta_uvarpro): drop dead inner class guard gg_beta_uvarpro already has a .default method (added with the wrapper), but its .uvarpro method still carried the unreachable `if (!inherits(object, "uvarpro")) stop(...)` guard. Remove it so the whole varPro family is consistent (no dead guards, class errors handled by .default). Behavior is unchanged — wrong-class input still errors via gg_beta_uvarpro.default. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent fc8544f commit 95900bd

9 files changed

Lines changed: 58 additions & 17 deletions

File tree

NAMESPACE

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,15 @@ S3method(calc_roc,randomForest)
2222
S3method(calc_roc,rfsrc)
2323
S3method(gg_beta_uvarpro,default)
2424
S3method(gg_beta_uvarpro,uvarpro)
25+
S3method(gg_beta_varpro,default)
2526
S3method(gg_beta_varpro,varpro)
2627
S3method(gg_brier,rfsrc)
2728
S3method(gg_error,randomForest)
2829
S3method(gg_error,randomForest.formula)
2930
S3method(gg_error,rfsrc)
31+
S3method(gg_isopro,default)
3032
S3method(gg_isopro,isopro)
33+
S3method(gg_ivarpro,default)
3134
S3method(gg_ivarpro,varpro)
3235
S3method(gg_rfsrc,randomForest)
3336
S3method(gg_rfsrc,rfsrc)

NEWS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@ Version: 3.4.0
33

44
ggRandomForests v3.4.0
55
======================
6+
* `gg_isopro()`, `gg_beta_varpro()`, and `gg_ivarpro()` now have `default` S3
7+
methods, so a wrong-class input gives a clear "expected a '<class>' object"
8+
error (naming the class it got) instead of R's generic "no applicable
9+
method". This makes the varPro-family wrappers consistent with
10+
`gg_beta_uvarpro()` / `gg_sdependent()`; the previously-unreachable inner
11+
class checks were removed.
612
* Fix: `gg_partial_rfsrc()` now computes partial dependence correctly for
713
`factor` predictors. It was passing factor *labels* as
814
`partial.values` to `randomForestSRC::partial.rfsrc()`, which imposes a

R/gg_beta_uvarpro.R

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ gg_beta_uvarpro.default <- function(object, ..., cutoff = NULL,
7777
#' @export
7878
gg_beta_uvarpro.uvarpro <- function(object, ..., cutoff = NULL,
7979
beta_fit = NULL) {
80-
if (!inherits(object, "uvarpro")) {
81-
stop("gg_beta_uvarpro: expected a 'uvarpro' object from varPro::uvarpro().",
82-
call. = FALSE)
83-
}
8480
.assert_scalar_numeric_or_null(cutoff, "cutoff", "gg_beta_uvarpro")
8581

8682
# Resolve the beta matrix (cache path)

R/gg_beta_varpro.R

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,17 @@ gg_beta_varpro <- function(object, ..., cutoff = NULL, beta_fit = NULL,
183183
UseMethod("gg_beta_varpro", object)
184184
}
185185

186+
#' @export
187+
gg_beta_varpro.default <- function(object, ..., cutoff = NULL,
188+
beta_fit = NULL, which_class = NULL) {
189+
stop("gg_beta_varpro: expected a 'varpro' object from varPro::varpro(); ",
190+
"got an object of class ", paste(class(object), collapse = "/"), ".",
191+
call. = FALSE)
192+
}
193+
186194
#' @export
187195
gg_beta_varpro.varpro <- function(object, ..., cutoff = NULL,
188196
beta_fit = NULL, which_class = NULL) {
189-
if (!inherits(object, "varpro")) {
190-
stop("gg_beta_varpro: expected a 'varpro' object from varPro::varpro().",
191-
call. = FALSE)
192-
}
193197
fam <- object$family
194198
if (!fam %in% c("regr", "class")) {
195199
stop(sprintf(

R/gg_isopro.R

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,12 +172,14 @@ gg_isopro <- function(object, ..., newdata = NULL) {
172172
}
173173

174174
#' @export
175-
gg_isopro.isopro <- function(object, ..., newdata = NULL) {
176-
if (!inherits(object, "isopro")) {
177-
stop("gg_isopro expects a 'isopro' object from varPro::isopro().",
178-
call. = FALSE)
179-
}
175+
gg_isopro.default <- function(object, ..., newdata = NULL) {
176+
stop("gg_isopro: expected an 'isopro' object from varPro::isopro(); ",
177+
"got an object of class ", paste(class(object), collapse = "/"), ".",
178+
call. = FALSE)
179+
}
180180

181+
#' @export
182+
gg_isopro.isopro <- function(object, ..., newdata = NULL) {
181183
ntree <- tryCatch(
182184
as.integer(object$isoforest$ntree),
183185
error = function(e) NA_integer_

R/gg_ivarpro.R

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,19 @@ gg_ivarpro <- function(object, ..., which_obs = NULL, which_class = NULL,
158158
UseMethod("gg_ivarpro", object)
159159
}
160160

161+
#' @export
162+
gg_ivarpro.default <- function(object, ..., which_obs = NULL,
163+
which_class = NULL, cutoff = NULL,
164+
ivarpro_fit = NULL) {
165+
stop("gg_ivarpro: expected a 'varpro' object from varPro::varpro(); ",
166+
"got an object of class ", paste(class(object), collapse = "/"), ".",
167+
call. = FALSE)
168+
}
169+
161170
#' @export
162171
gg_ivarpro.varpro <- function(object, ..., which_obs = NULL,
163172
which_class = NULL, cutoff = NULL,
164173
ivarpro_fit = NULL) {
165-
if (!inherits(object, "varpro")) {
166-
stop("gg_ivarpro: expected a 'varpro' object from varPro::varpro().",
167-
call. = FALSE)
168-
}
169174
fam <- object$family
170175
if (!fam %in% c("regr", "class")) {
171176
stop(sprintf(

tests/testthat/test_gg_beta_varpro.R

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,10 @@
1+
test_that("gg_beta_varpro: non-varpro input errors via the default method", {
2+
# No varPro fit needed — the default method fires before any varpro work.
3+
expect_error(gg_beta_varpro(data.frame(a = 1:3)),
4+
"expected a 'varpro' object")
5+
expect_error(gg_beta_varpro(1:3), "got an object of class integer")
6+
})
7+
18
test_that("gg_beta_varpro returns the expected tidy shape", {
29
b <- .beta_fit_mtcars()
310
v <- .varpro_mtcars()

tests/testthat/test_gg_isopro.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ make_iso_fit <- function(seed = 1L, method = "rnd", ntree = 25, sampsize = 16) {
1919
)
2020
}
2121

22+
test_that("gg_isopro: non-isopro input errors via the default method", {
23+
# No varPro fit needed — the default method fires before any isopro work,
24+
# so this runs everywhere (incl. CRAN).
25+
expect_error(gg_isopro(data.frame(a = 1:3)),
26+
"expected an 'isopro' object")
27+
expect_error(gg_isopro(1:3),
28+
"got an object of class integer")
29+
})
30+
2231
test_that("gg_isopro: returns gg_isopro data.frame with correct columns", {
2332
fit <- make_iso_fit()
2433
gg <- gg_isopro(fit)

tests/testthat/test_gg_ivarpro.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
# ---- Dispatch --------------------------------------------------------------
2+
3+
test_that("gg_ivarpro: non-varpro input errors via the default method", {
4+
# No varPro fit needed — the default method fires before any varpro work.
5+
expect_error(gg_ivarpro(data.frame(a = 1:3)),
6+
"expected a 'varpro' object")
7+
expect_error(gg_ivarpro(1:3), "got an object of class integer")
8+
})
9+
110
# ---- Shape ----------------------------------------------------------------
211

312
test_that("gg_ivarpro regression returns long-format tidy frame", {

0 commit comments

Comments
 (0)