fix(vignettes): static PD surfaces + 96-dpi figures to cut install size#110
Merged
Conversation
The regression and survival partial-dependence surfaces were interactive plotly widgets; self-contained quarto inlined plotly.js (~3.5 MB) into each vignette HTML, and figures rendered at retina 2x. Installed size was 17.1 MB (doc 16.3 MB), well over CRAN's 5 MB guideline. Replace both surfaces with static ggplot2 heat maps, set fig-format png / fig-dpi 96 in all four vignettes, and drop the now-unused plotly Suggests. Installed size drops to ~5.5 MB (doc 4.7 MB); source tarball 9.0 -> 3.7 MB. R CMD check --as-cran (with manual, ggraph present): pending confirmation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
=======================================
Coverage 87.04% 87.04%
=======================================
Files 44 44
Lines 3938 3938
=======================================
Hits 3428 3428
Misses 510 510 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR reduces the built vignette footprint to better align with CRAN size guidelines by removing embedded Plotly JS payloads and lowering rendered figure density in Quarto HTML output.
Changes:
- Replaced Plotly-based 3D partial-dependence surfaces in regression/survival vignettes with static
ggplot2heatmaps. - Set Quarto HTML figure defaults to
fig-format: pngandfig-dpi: 96across all four vignettes. - Dropped
plotlyfromSuggestsand documented the change inNEWS.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/varpro.qmd | Sets Quarto HTML figure output to PNG at 96 dpi. |
| vignettes/ggRandomForests.qmd | Sets Quarto HTML figure output to PNG at 96 dpi. |
| vignettes/ggRandomForests-survival.qmd | Replaces Plotly surface with static ggplot2 heatmap and updates related prose; sets PNG/96 dpi. |
| vignettes/ggRandomForests-regression.qmd | Replaces Plotly surface with static ggplot2 heatmap and updates related prose; sets PNG/96 dpi. |
| NEWS.md | Adds v3.1.0 note about vignette surface change, dpi change, and removal of Plotly dependency. |
| DESCRIPTION | Removes plotly from Suggests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ehrlinger
added a commit
that referenced
this pull request
Jun 11, 2026
…#117) * docs: v3.1.0 documentation sweep + gg_vimp fix (CRAN release) (#109) * docs: v3.1.0 documentation-sweep design spec * docs: v3.1.0 spec — fix bugs surfaced by canonical-source reconciliation (with severity triage) * docs: v3.1.0 doc-sweep implementation plan * docs: deepen varPro-family roxygen (release-rules framing, vimp-vs-varpro) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add the gg_vimp-vs-gg_varpro distinction to gg_vimp (Task 2 fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: deepen rfsrc partial/survival/rfsrc roxygen (ensemble + partial-dependence framing) * docs: address Task 2 review (drop invented first-person in gg_survival; non-positive VIMP wording) * docs: voice/drift cleanup on remaining roxygen topics Remove stale yvar @return item from gg_roc — the function returns sens/spec/pct (from calc_roc), not a yvar column per observation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): deepen varpro — release-rules framing, refs Deepens all prose sections of varpro.qmd with release-rules/guided-splitting framing; adds Lee:2021 bib key (AOS 49:4) cited in PBC section; adds varProtools URL in Further Reading. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): regression — vimp-vs-varpro contrast, rfsrc ref Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): survival — rfsrc ensemble framing, ref Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): intro — voice/drift pass * docs(comments): correctness + gap pass on R/ source Fix a misleading AUC trapezoidal-rule comment in calc_roc.R (the old text introduced Δ(FPR) but the code uses Δspec; reworded to state the equivalence plainly). Remove a stale varPro-specific note from the categorical branch of gg_partial.R (plot.variable output has no connection to varPro one-hot encoding). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(NEWS): open v3.1.0 development heading (version unchanged) * docs(vignette): trim em-dashes in varpro per voice standard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(PR#109 review): gg_vimp positive flag (VIMP vs vimp case), Brier 0.25 precision * chore(release): prepare v3.1.0 for CRAN Bump DESCRIPTION + NEWS to 3.1.0 (CRAN never saw v3.0.0; jump from 2.7.3 is intentional) and finalize the v3.1.0 NEWS heading. Trim em-dashes and right-arrows from roxygen and code comments per the package voice standard (68 replacements across R/), then re-document so man/*.Rd carries no raw non-ASCII into the PDF manual build. Rewrite cran-comments.md for the 2.7.3 -> 3.1.0 submission: fold in the v3.0.0 feature layer, correct the local test env (R 4.6.0/darwin23). R CMD check --as-cran (with manual build, ggraph present): 0/0/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): correct v3.0.0 history v3.0.0 was submitted but did not complete the CRAN review cycle; 3.1.0 supersedes it. Prior wording said it was never submitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): note v3.0.0 pretests were clean, hold was heuristic Per the release handoff: tell the CRAN reviewer the 2026-05-28 v3.0.0 submission cleared incoming pretests on Windows + Debian (0/0/0) and the auto-hold looked like a version-jump/Depends-to-Imports heuristic, not a defect, in case the same heuristic flags v3.1.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(dev/plans): mark v3.0.0-held release mechanics as superseded The plan/design docs described the held workflow (keep Version 3.0.0, merge only after CRAN accepts v3.0.0, cut 3.1.0 at a post-acceptance RC). v3.0.0 lapsed un-reviewed, so we ship 3.1.0 directly. Banners note this; the documentation-content plan is unchanged. Addresses Copilot review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(vignettes): static PD surfaces + 96-dpi figures to cut install size (#110) The regression and survival partial-dependence surfaces were interactive plotly widgets; self-contained quarto inlined plotly.js (~3.5 MB) into each vignette HTML, and figures rendered at retina 2x. Installed size was 17.1 MB (doc 16.3 MB), well over CRAN's 5 MB guideline. Replace both surfaces with static ggplot2 heat maps, set fig-format png / fig-dpi 96 in all four vignettes, and drop the now-unused plotly Suggests. Installed size drops to ~5.5 MB (doc 4.7 MB); source tarball 9.0 -> 3.7 MB. R CMD check --as-cran (with manual, ggraph present): pending confirmation. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * docs(examples): \donttest the slow plot.gg_variable example sections (#113) win-builder R-oldrelease flagged the plot.gg_variable example at 10.33s elapsed (just over CRAN's 10s; under 10s on release/devel). Wrap the loess-heavy regression panel plot and the full survival section (veteran forest + multi-time variable/panel plots) in \donttest so they are excluded from the timed example run; the fast classification + basic regression plots still run. No behaviour change. R CMD check --as-cran: examples [14s] OK, examples --run-donttest [28s] OK, Status: OK (0/0/0). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * Cut CRAN overall check time below 10 min (#114) * perf(check): cut CRAN overall check time below 10 min CRAN flagged the 3.1.0 submission's overall check time (13 min > 10 min), driven by the vignette rebuild (331s) and tests (209s). Reduce both per Uwe Ligges' suggested levers (toy data / fewer iterations / precomputed results), with no change to test coverage or vignette content. Vignettes: - regression: Boston forest ntree 200, PD-surface grid 25 -> 10 - survival: impute ntree 100, forest ntree 150, PD-surface grid 25 -> 8 - varpro: the three gg_partial_varpro() calls (11-17s each) and the Boston beta.varpro() fit (~3s) -- the bulk of that vignette -- are precomputed offline by precompute_varpro.R and loaded from varpro_precomputed.rds (167 KB, xz), with an automatic live-computation fallback if absent. Tests: - test_gg_udependent memoised varPro::get.beta.entropy() (~1.5s, a pure function of the fit) per argument signature instead of recomputing it once per test (this file was ~24s of the suite, now ~9s). Verified: R CMD check --as-cran with manual is OK (0/0/0); local vignette rebuild 33s and tests 28s (were 331s/209s on CRAN's r-devel-windows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review: address Copilot feedback on the check-time PR - varpro.qmd: load the precompute via tryCatch(readRDS) so a missing OR unreadable .rds falls back to live computation instead of erroring. - precompute_varpro.R: mirror the vignette's requireNamespace/pkgload fallback instead of bare library(ggRandomForests), so the script runs in a fresh clone before the package is installed. - test_gg_udependent.R: make make_ggu() warning suppression opt-in (.quiet = FALSE by default); pass .quiet = TRUE only to the empty-graph (threshold = 999) cases that legitimately warn, so an unexpected warning on any other call still fails the test. Verified: test_gg_udependent 19 tests, 0 fail / 0 warn; varpro vignette renders in 20s with 0 errors (precompute still loaded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review(#114): search both paths for varpro_precomputed.rds The precomputed-load chunk read only the cwd-relative 'varpro_precomputed.rds'; depending on how Quarto sets the working directory during R CMD check this could miss the file and silently fall back to (slower) live computation. Search both the vignette-dir and package-root locations before the live fallback. Addresses Copilot review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore: open 3.1.0.9000 dev cycle after the CRAN release (#115) v3.1.0 accepted to CRAN (2026-06-11). Bump main to the post-release .9000 dev version (DESCRIPTION + NEWS, dual update so the news-version test sees the DESCRIPTION version), and record the release submission in CRAN-SUBMISSION (SHA a7d8052). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * review(#117): deterministic gg_vimp positive-flag test; MASS guard in precompute Copilot review on the forward-merge: - gg_vimp single-outcome regression test was non-deterministic (no seed) and only checked the invariant when a non-positive VIMP happened to exist. Add set.seed + a zero-variance `const` predictor (VIMP exactly 0 on every platform) and assert positive == (VIMP > 0) for all rows, plus any(!positive) to guarantee the bug condition is exercised. - vignettes/precompute_varpro.R now checks requireNamespace("MASS") with a clear message before loading the Boston data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
ehrlinger
added a commit
that referenced
this pull request
Jun 13, 2026
* docs: v3.1.0 documentation sweep + gg_vimp fix (CRAN release) (#109) * docs: v3.1.0 documentation-sweep design spec * docs: v3.1.0 spec — fix bugs surfaced by canonical-source reconciliation (with severity triage) * docs: v3.1.0 doc-sweep implementation plan * docs: deepen varPro-family roxygen (release-rules framing, vimp-vs-varpro) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add the gg_vimp-vs-gg_varpro distinction to gg_vimp (Task 2 fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: deepen rfsrc partial/survival/rfsrc roxygen (ensemble + partial-dependence framing) * docs: address Task 2 review (drop invented first-person in gg_survival; non-positive VIMP wording) * docs: voice/drift cleanup on remaining roxygen topics Remove stale yvar @return item from gg_roc — the function returns sens/spec/pct (from calc_roc), not a yvar column per observation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): deepen varpro — release-rules framing, refs Deepens all prose sections of varpro.qmd with release-rules/guided-splitting framing; adds Lee:2021 bib key (AOS 49:4) cited in PBC section; adds varProtools URL in Further Reading. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): regression — vimp-vs-varpro contrast, rfsrc ref Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): survival — rfsrc ensemble framing, ref Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): intro — voice/drift pass * docs(comments): correctness + gap pass on R/ source Fix a misleading AUC trapezoidal-rule comment in calc_roc.R (the old text introduced Δ(FPR) but the code uses Δspec; reworded to state the equivalence plainly). Remove a stale varPro-specific note from the categorical branch of gg_partial.R (plot.variable output has no connection to varPro one-hot encoding). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(NEWS): open v3.1.0 development heading (version unchanged) * docs(vignette): trim em-dashes in varpro per voice standard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(PR#109 review): gg_vimp positive flag (VIMP vs vimp case), Brier 0.25 precision * chore(release): prepare v3.1.0 for CRAN Bump DESCRIPTION + NEWS to 3.1.0 (CRAN never saw v3.0.0; jump from 2.7.3 is intentional) and finalize the v3.1.0 NEWS heading. Trim em-dashes and right-arrows from roxygen and code comments per the package voice standard (68 replacements across R/), then re-document so man/*.Rd carries no raw non-ASCII into the PDF manual build. Rewrite cran-comments.md for the 2.7.3 -> 3.1.0 submission: fold in the v3.0.0 feature layer, correct the local test env (R 4.6.0/darwin23). R CMD check --as-cran (with manual build, ggraph present): 0/0/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): correct v3.0.0 history v3.0.0 was submitted but did not complete the CRAN review cycle; 3.1.0 supersedes it. Prior wording said it was never submitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): note v3.0.0 pretests were clean, hold was heuristic Per the release handoff: tell the CRAN reviewer the 2026-05-28 v3.0.0 submission cleared incoming pretests on Windows + Debian (0/0/0) and the auto-hold looked like a version-jump/Depends-to-Imports heuristic, not a defect, in case the same heuristic flags v3.1.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(dev/plans): mark v3.0.0-held release mechanics as superseded The plan/design docs described the held workflow (keep Version 3.0.0, merge only after CRAN accepts v3.0.0, cut 3.1.0 at a post-acceptance RC). v3.0.0 lapsed un-reviewed, so we ship 3.1.0 directly. Banners note this; the documentation-content plan is unchanged. Addresses Copilot review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(vignettes): static PD surfaces + 96-dpi figures to cut install size (#110) The regression and survival partial-dependence surfaces were interactive plotly widgets; self-contained quarto inlined plotly.js (~3.5 MB) into each vignette HTML, and figures rendered at retina 2x. Installed size was 17.1 MB (doc 16.3 MB), well over CRAN's 5 MB guideline. Replace both surfaces with static ggplot2 heat maps, set fig-format png / fig-dpi 96 in all four vignettes, and drop the now-unused plotly Suggests. Installed size drops to ~5.5 MB (doc 4.7 MB); source tarball 9.0 -> 3.7 MB. R CMD check --as-cran (with manual, ggraph present): pending confirmation. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * docs(examples): \donttest the slow plot.gg_variable example sections (#113) win-builder R-oldrelease flagged the plot.gg_variable example at 10.33s elapsed (just over CRAN's 10s; under 10s on release/devel). Wrap the loess-heavy regression panel plot and the full survival section (veteran forest + multi-time variable/panel plots) in \donttest so they are excluded from the timed example run; the fast classification + basic regression plots still run. No behaviour change. R CMD check --as-cran: examples [14s] OK, examples --run-donttest [28s] OK, Status: OK (0/0/0). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * Cut CRAN overall check time below 10 min (#114) * perf(check): cut CRAN overall check time below 10 min CRAN flagged the 3.1.0 submission's overall check time (13 min > 10 min), driven by the vignette rebuild (331s) and tests (209s). Reduce both per Uwe Ligges' suggested levers (toy data / fewer iterations / precomputed results), with no change to test coverage or vignette content. Vignettes: - regression: Boston forest ntree 200, PD-surface grid 25 -> 10 - survival: impute ntree 100, forest ntree 150, PD-surface grid 25 -> 8 - varpro: the three gg_partial_varpro() calls (11-17s each) and the Boston beta.varpro() fit (~3s) -- the bulk of that vignette -- are precomputed offline by precompute_varpro.R and loaded from varpro_precomputed.rds (167 KB, xz), with an automatic live-computation fallback if absent. Tests: - test_gg_udependent memoised varPro::get.beta.entropy() (~1.5s, a pure function of the fit) per argument signature instead of recomputing it once per test (this file was ~24s of the suite, now ~9s). Verified: R CMD check --as-cran with manual is OK (0/0/0); local vignette rebuild 33s and tests 28s (were 331s/209s on CRAN's r-devel-windows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review: address Copilot feedback on the check-time PR - varpro.qmd: load the precompute via tryCatch(readRDS) so a missing OR unreadable .rds falls back to live computation instead of erroring. - precompute_varpro.R: mirror the vignette's requireNamespace/pkgload fallback instead of bare library(ggRandomForests), so the script runs in a fresh clone before the package is installed. - test_gg_udependent.R: make make_ggu() warning suppression opt-in (.quiet = FALSE by default); pass .quiet = TRUE only to the empty-graph (threshold = 999) cases that legitimately warn, so an unexpected warning on any other call still fails the test. Verified: test_gg_udependent 19 tests, 0 fail / 0 warn; varpro vignette renders in 20s with 0 errors (precompute still loaded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review(#114): search both paths for varpro_precomputed.rds The precomputed-load chunk read only the cwd-relative 'varpro_precomputed.rds'; depending on how Quarto sets the working directory during R CMD check this could miss the file and silently fall back to (slower) live computation. Search both the vignette-dir and package-root locations before the live fallback. Addresses Copilot review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore: open 3.1.0.9000 dev cycle after the CRAN release (#115) v3.1.0 accepted to CRAN (2026-06-11). Bump main to the post-release .9000 dev version (DESCRIPTION + NEWS, dual update so the news-version test sees the DESCRIPTION version), and record the release submission in CRAN-SUBMISSION (SHA a7d8052). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): v3.1.1 — clear gcc-UBSAN additional issue (varPro test skip + vignette precompute) (#119) * fix(cran): skip_on_cran the varPro tests to avoid upstream UBSAN (v3.1.1) CRAN's gcc-UBSAN additional check flagged 3.1.0 with a 0-length array access in randomForestSRC's compiled rfsrcGrow (entry.c:184), reached when varPro::varpro()/beta.varpro() grow a forest in our tests. ggRandomForests is pure R (NeedsCompilation: no) — the overflow is in a dependency, surfaced by our tests. Gate every varPro forest-growing test fixture/builder with testthat::skip_on_cran() so CRAN's machines (incl. gcc-UBSAN) never run them. No package code changed; the tests still run on CI and locally. Bump to 3.1.1; NEWS + cran-comments explain the fix. Upstream reported. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): note the benign 'days since last update' NOTE Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): precompute all varPro vignette fits to avoid upstream UBSAN The varpro vignette grew ~10 varPro forests live (varpro/uvarpro/isopro/ ivarpro/beta.varpro), each reaching randomForestSRC's rfsrcGrow rule-grow path that trips the gcc-UBSAN "0-length array" report (entry.c:184, a length-0 yvar.wt decremented to an out-of-bounds pointer). Cache every fit in vignettes/varpro_precomputed.rds and load it with a live fallback, so R CMD check performs no live varPro grow. Strip the unused embedded forests ($rf, $isoforest, redundant ivarpro attrs) before saving — validated that every gg_* wrapper call returns output identical to the un-stripped object — keeping the file at 414 KB (tarball 4.13 MB, under CRAN's 5 MB limit). R CMD check --as-cran (with manual): 0 errors, 0 warnings, 1 NOTE (days-since-update, expected). Overall check time 2:43. dev/randomForestSRC-ubsan-report.md records the upstream root cause + suggested patch (maintainers are aware and staging a fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(vignette): satisfy lintr (brace/semicolon) in fallback chunks Lint CI flagged 16 brace_linter/semicolon_linter issues in the vignette hardening: - varpro.qmd: the load-with-fallback chunks braced the `if` branch but not `else` (`} else .vp$x`); brace both branches to match the existing precomputed chunks. - precompute_varpro.R: rewrite the one-line .strip_* helpers (compound semicolons + inline braces) as multi-line. Behaviour-preserving; lint_package() now returns 0. The shipped varpro_precomputed.rds is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: address Copilot review — accurate CI claims, scoped strip comments Copilot flagged that skip_on_cran() also skips under R CMD check in CI (NOT_CRAN unset in the workflows), so the varPro tests run nowhere in CI: - NEWS.md / cran-comments.md no longer claim the tests "run on CI"; they state the tests run locally (devtools::test()) and are skipped under R CMD check, including the CI check jobs. (Restoring CI coverage via NOT_CRAN=true is deferred to 3.1.2, coupled with the issue #118 fix so the intermittent survival gg_varpro test doesn't flake CI.) - precompute_varpro.R: scope the "forests unused" comments — the survival C-path gg_partial_varpro() and gg_isopro(newdata=) DO use $rf/$isoforest, but the vignette never invokes those on a stripped object (pd_pbc cached, gg_isopro training-path only). No behaviour change; lint_package() = 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: set NOT_CRAN=true so varPro tests run in CI (Copilot review) Copilot noted skip_on_cran() also skips the varPro tests under R CMD check in CI (NOT_CRAN unset), so they ran nowhere. Set NOT_CRAN=true in all check/coverage workflows to restore that coverage. Safe: CI is not a sanitizer build, so the upstream randomForestSRC UBSAN path is harmless here; only CRAN's own check machines (NOT_CRAN unset) skip them, which is what avoids the gcc-UBSAN additional issue. The CI-run varPro tests cover regression + classification only (no survival), so issue #118 cannot flake CI. Verified locally under NOT_CRAN=true: 0 failures across the varPro suite. NEWS.md / cran-comments.md restore the accurate "runs in CI and locally; skipped only on CRAN" claim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): v3.1.2 — skip only isopro(method="unsupv"), the sole gcc-UBSAN trigger (#122) * fix(cran): v3.1.2 — skip_on_cran the isopro fixture (clear gcc-UBSAN) v3.1.1 guarded the varPro forest-growing test fixtures with skip_on_cran() but missed make_iso_fit() in tests/testthat/test_gg_isopro.R. It was gated only by skip_if_not_installed("varPro"), and varPro IS installed on CRAN's check machines — so the ~23 isopro tests still ran there. varPro::isopro() grows an *unsupervised* isolation forest (do.call("rfsrc", ...) with the unsupv family → yvar.wt length 0). That is the exact path that trips randomForestSRC's gcc-UBSAN report at entry.c:184 (unconditional RF_yWeight-- on a 0-length weight vector → out-of-bounds pointer, UB). It was the one unsupervised grow v3.1.1 left unguarded, so the additional check re-flagged the issue against 3.1.1. Fix: add testthat::skip_on_cran() to make_iso_fit(), matching the other varPro fixtures. ggRandomForests is pure R (NeedsCompilation: no); package code is unchanged. The isopro tests still run in CI and locally (NOT_CRAN=true). Verified: R CMD check --as-cran (with manual) → 0 errors, 0 warnings, 1 NOTE (days-since-update), 2.6 min. test-all.Rout shows all 23 isopro tests in the "On CRAN" skip list, so the unsupervised grow no longer executes under the check. Bumps DESCRIPTION + NEWS to 3.1.2; cran-comments updated. Upstream fix is randomForestSRC commit 92ec283 (not yet on CRAN). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): narrow the UBSAN skip to only isopro(method="unsupv") Empirically pinned the gcc-UBSAN trigger with -fsanitize=undefined on CRAN's randomForestSRC 3.6.2: of every varPro/rfsrc grow in the suite, ONLY varPro::isopro(method="unsupv") fires entry.c:184. It is the sole grow with a length-0 yvar.wt (unsupervised family). Everything else is supervised or synthetic-supervised and UBSAN-clean: - varpro / ivarpro / beta.varpro: real Y -> non-empty yvar.wt - isopro(method="rnd"): synthetic supervised forest - uvarpro: grows yxyz123 ~ . with an rnorm response (synthetic supervised) So: - make_iso_fit() now skip_on_cran() ONLY when method == "unsupv" (the rnd isopro tests run on CRAN again). - Reverted v3.1.1's blanket skip_on_cran() on the varpro/uvarpro/ivarpro/ beta fixtures (helper-varpro-fixtures.R, test_gg_varpro.R, test_gg_udependent.R) -> ~126 varPro tests run on CRAN again. Verified: - Full suite under CRAN conditions (NOT_CRAN unset) against a GCC-equivalent UBSAN rfsrc (-fsanitize=undefined -fno-sanitize=float-cast-overflow): 0 runtime errors, FAIL 0 / PASS 1093 / SKIP 7. Only 2 tests skip On CRAN (isopro-unsupv + the pre-existing slow ivarpro test). - R CMD check --as-cran (with manual): 0 errors, 0 warnings, 1 NOTE (days-since-update); overall 2.68 min, well under the 10-min budget. Note: a local clang UBSAN build also surfaces partial.c:92 (a (uint)NaN float-cast in test_gg_partial_varpro's categorical survival partial dependence). CRAN does not see it: GCC's -fsanitize=undefined excludes float-cast-overflow (clang's includes it). Confirmed by rebuilding with that check disabled. Separate latent upstream issue, not a CRAN blocker. NEWS + cran-comments updated to describe the narrowed fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(isopro): namespace-qualify skip_* in make_iso_fit Use testthat::skip_on_cran()/skip_if_not_installed() so the helper does not depend on testthat being attached. Addresses a Copilot review note on #122; qualifies both calls for internal consistency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): correct NOTE disposition + narrowed-skip wording R CMD check --as-cran returns 1 NOTE (days since last update), not 0; only the single unsupervised isopro test skips on CRAN now (all other varPro tests run). Genericise the day count so the submitted comment is not stale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore(cran): record 3.1.2 submission (#123) submit_cran() recorded the submitted commit (SHA 83d8021, the merged #122) and timestamp 2026-06-13. ggRandomForests 3.1.2 submitted to CRAN to clear the gcc-UBSAN additional issue. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * review: restore MASS guard in precompute_varpro.R Addresses Copilot review on #126: MASS is only in Suggests, so guard the `data("Boston", package = "MASS")` call with an explicit requireNamespace("MASS") check + clear stop, instead of failing with a non-obvious error in a minimal dev environment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(gg_vimp): make single-outcome positive-flag test deterministic The forward-merge kept dev's version of this test, which relied on a zero-variance 'const' predictor having non-positive VIMP. Permutation VIMP of a weak/constant variable can land slightly >0 (rfsrc may also drop a constant column), so `any(!positive)` was FALSE on R-release -> CI failure. Inject one non-positive importance (`rf$importance[1] <- -1`) so the test reliably exercises the bug condition on every platform; the invariant assertion (positive == VIMP>0) is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
ehrlinger
added a commit
that referenced
this pull request
Jun 23, 2026
) * docs: v3.1.0 documentation sweep + gg_vimp fix (CRAN release) (#109) * docs: v3.1.0 documentation-sweep design spec * docs: v3.1.0 spec — fix bugs surfaced by canonical-source reconciliation (with severity triage) * docs: v3.1.0 doc-sweep implementation plan * docs: deepen varPro-family roxygen (release-rules framing, vimp-vs-varpro) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add the gg_vimp-vs-gg_varpro distinction to gg_vimp (Task 2 fix) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: deepen rfsrc partial/survival/rfsrc roxygen (ensemble + partial-dependence framing) * docs: address Task 2 review (drop invented first-person in gg_survival; non-positive VIMP wording) * docs: voice/drift cleanup on remaining roxygen topics Remove stale yvar @return item from gg_roc — the function returns sens/spec/pct (from calc_roc), not a yvar column per observation. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): deepen varpro — release-rules framing, refs Deepens all prose sections of varpro.qmd with release-rules/guided-splitting framing; adds Lee:2021 bib key (AOS 49:4) cited in PBC section; adds varProtools URL in Further Reading. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): regression — vimp-vs-varpro contrast, rfsrc ref Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): survival — rfsrc ensemble framing, ref Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(vignette): intro — voice/drift pass * docs(comments): correctness + gap pass on R/ source Fix a misleading AUC trapezoidal-rule comment in calc_roc.R (the old text introduced Δ(FPR) but the code uses Δspec; reworded to state the equivalence plainly). Remove a stale varPro-specific note from the categorical branch of gg_partial.R (plot.variable output has no connection to varPro one-hot encoding). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(NEWS): open v3.1.0 development heading (version unchanged) * docs(vignette): trim em-dashes in varpro per voice standard Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(PR#109 review): gg_vimp positive flag (VIMP vs vimp case), Brier 0.25 precision * chore(release): prepare v3.1.0 for CRAN Bump DESCRIPTION + NEWS to 3.1.0 (CRAN never saw v3.0.0; jump from 2.7.3 is intentional) and finalize the v3.1.0 NEWS heading. Trim em-dashes and right-arrows from roxygen and code comments per the package voice standard (68 replacements across R/), then re-document so man/*.Rd carries no raw non-ASCII into the PDF manual build. Rewrite cran-comments.md for the 2.7.3 -> 3.1.0 submission: fold in the v3.0.0 feature layer, correct the local test env (R 4.6.0/darwin23). R CMD check --as-cran (with manual build, ggraph present): 0/0/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): correct v3.0.0 history v3.0.0 was submitted but did not complete the CRAN review cycle; 3.1.0 supersedes it. Prior wording said it was never submitted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): note v3.0.0 pretests were clean, hold was heuristic Per the release handoff: tell the CRAN reviewer the 2026-05-28 v3.0.0 submission cleared incoming pretests on Windows + Debian (0/0/0) and the auto-hold looked like a version-jump/Depends-to-Imports heuristic, not a defect, in case the same heuristic flags v3.1.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(dev/plans): mark v3.0.0-held release mechanics as superseded The plan/design docs described the held workflow (keep Version 3.0.0, merge only after CRAN accepts v3.0.0, cut 3.1.0 at a post-acceptance RC). v3.0.0 lapsed un-reviewed, so we ship 3.1.0 directly. Banners note this; the documentation-content plan is unchanged. Addresses Copilot review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(vignettes): static PD surfaces + 96-dpi figures to cut install size (#110) The regression and survival partial-dependence surfaces were interactive plotly widgets; self-contained quarto inlined plotly.js (~3.5 MB) into each vignette HTML, and figures rendered at retina 2x. Installed size was 17.1 MB (doc 16.3 MB), well over CRAN's 5 MB guideline. Replace both surfaces with static ggplot2 heat maps, set fig-format png / fig-dpi 96 in all four vignettes, and drop the now-unused plotly Suggests. Installed size drops to ~5.5 MB (doc 4.7 MB); source tarball 9.0 -> 3.7 MB. R CMD check --as-cran (with manual, ggraph present): pending confirmation. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * docs(examples): \donttest the slow plot.gg_variable example sections (#113) win-builder R-oldrelease flagged the plot.gg_variable example at 10.33s elapsed (just over CRAN's 10s; under 10s on release/devel). Wrap the loess-heavy regression panel plot and the full survival section (veteran forest + multi-time variable/panel plots) in \donttest so they are excluded from the timed example run; the fast classification + basic regression plots still run. No behaviour change. R CMD check --as-cran: examples [14s] OK, examples --run-donttest [28s] OK, Status: OK (0/0/0). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * Cut CRAN overall check time below 10 min (#114) * perf(check): cut CRAN overall check time below 10 min CRAN flagged the 3.1.0 submission's overall check time (13 min > 10 min), driven by the vignette rebuild (331s) and tests (209s). Reduce both per Uwe Ligges' suggested levers (toy data / fewer iterations / precomputed results), with no change to test coverage or vignette content. Vignettes: - regression: Boston forest ntree 200, PD-surface grid 25 -> 10 - survival: impute ntree 100, forest ntree 150, PD-surface grid 25 -> 8 - varpro: the three gg_partial_varpro() calls (11-17s each) and the Boston beta.varpro() fit (~3s) -- the bulk of that vignette -- are precomputed offline by precompute_varpro.R and loaded from varpro_precomputed.rds (167 KB, xz), with an automatic live-computation fallback if absent. Tests: - test_gg_udependent memoised varPro::get.beta.entropy() (~1.5s, a pure function of the fit) per argument signature instead of recomputing it once per test (this file was ~24s of the suite, now ~9s). Verified: R CMD check --as-cran with manual is OK (0/0/0); local vignette rebuild 33s and tests 28s (were 331s/209s on CRAN's r-devel-windows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review: address Copilot feedback on the check-time PR - varpro.qmd: load the precompute via tryCatch(readRDS) so a missing OR unreadable .rds falls back to live computation instead of erroring. - precompute_varpro.R: mirror the vignette's requireNamespace/pkgload fallback instead of bare library(ggRandomForests), so the script runs in a fresh clone before the package is installed. - test_gg_udependent.R: make make_ggu() warning suppression opt-in (.quiet = FALSE by default); pass .quiet = TRUE only to the empty-graph (threshold = 999) cases that legitimately warn, so an unexpected warning on any other call still fails the test. Verified: test_gg_udependent 19 tests, 0 fail / 0 warn; varpro vignette renders in 20s with 0 errors (precompute still loaded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * review(#114): search both paths for varpro_precomputed.rds The precomputed-load chunk read only the cwd-relative 'varpro_precomputed.rds'; depending on how Quarto sets the working directory during R CMD check this could miss the file and silently fall back to (slower) live computation. Search both the vignette-dir and package-root locations before the live fallback. Addresses Copilot review. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore: open 3.1.0.9000 dev cycle after the CRAN release (#115) v3.1.0 accepted to CRAN (2026-06-11). Bump main to the post-release .9000 dev version (DESCRIPTION + NEWS, dual update so the news-version test sees the DESCRIPTION version), and record the release submission in CRAN-SUBMISSION (SHA a7d8052). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): v3.1.1 — clear gcc-UBSAN additional issue (varPro test skip + vignette precompute) (#119) * fix(cran): skip_on_cran the varPro tests to avoid upstream UBSAN (v3.1.1) CRAN's gcc-UBSAN additional check flagged 3.1.0 with a 0-length array access in randomForestSRC's compiled rfsrcGrow (entry.c:184), reached when varPro::varpro()/beta.varpro() grow a forest in our tests. ggRandomForests is pure R (NeedsCompilation: no) — the overflow is in a dependency, surfaced by our tests. Gate every varPro forest-growing test fixture/builder with testthat::skip_on_cran() so CRAN's machines (incl. gcc-UBSAN) never run them. No package code changed; the tests still run on CI and locally. Bump to 3.1.1; NEWS + cran-comments explain the fix. Upstream reported. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): note the benign 'days since last update' NOTE Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): precompute all varPro vignette fits to avoid upstream UBSAN The varpro vignette grew ~10 varPro forests live (varpro/uvarpro/isopro/ ivarpro/beta.varpro), each reaching randomForestSRC's rfsrcGrow rule-grow path that trips the gcc-UBSAN "0-length array" report (entry.c:184, a length-0 yvar.wt decremented to an out-of-bounds pointer). Cache every fit in vignettes/varpro_precomputed.rds and load it with a live fallback, so R CMD check performs no live varPro grow. Strip the unused embedded forests ($rf, $isoforest, redundant ivarpro attrs) before saving — validated that every gg_* wrapper call returns output identical to the un-stripped object — keeping the file at 414 KB (tarball 4.13 MB, under CRAN's 5 MB limit). R CMD check --as-cran (with manual): 0 errors, 0 warnings, 1 NOTE (days-since-update, expected). Overall check time 2:43. dev/randomForestSRC-ubsan-report.md records the upstream root cause + suggested patch (maintainers are aware and staging a fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(vignette): satisfy lintr (brace/semicolon) in fallback chunks Lint CI flagged 16 brace_linter/semicolon_linter issues in the vignette hardening: - varpro.qmd: the load-with-fallback chunks braced the `if` branch but not `else` (`} else .vp$x`); brace both branches to match the existing precomputed chunks. - precompute_varpro.R: rewrite the one-line .strip_* helpers (compound semicolons + inline braces) as multi-line. Behaviour-preserving; lint_package() now returns 0. The shipped varpro_precomputed.rds is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: address Copilot review — accurate CI claims, scoped strip comments Copilot flagged that skip_on_cran() also skips under R CMD check in CI (NOT_CRAN unset in the workflows), so the varPro tests run nowhere in CI: - NEWS.md / cran-comments.md no longer claim the tests "run on CI"; they state the tests run locally (devtools::test()) and are skipped under R CMD check, including the CI check jobs. (Restoring CI coverage via NOT_CRAN=true is deferred to 3.1.2, coupled with the issue #118 fix so the intermittent survival gg_varpro test doesn't flake CI.) - precompute_varpro.R: scope the "forests unused" comments — the survival C-path gg_partial_varpro() and gg_isopro(newdata=) DO use $rf/$isoforest, but the vignette never invokes those on a stripped object (pd_pbc cached, gg_isopro training-path only). No behaviour change; lint_package() = 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci: set NOT_CRAN=true so varPro tests run in CI (Copilot review) Copilot noted skip_on_cran() also skips the varPro tests under R CMD check in CI (NOT_CRAN unset), so they ran nowhere. Set NOT_CRAN=true in all check/coverage workflows to restore that coverage. Safe: CI is not a sanitizer build, so the upstream randomForestSRC UBSAN path is harmless here; only CRAN's own check machines (NOT_CRAN unset) skip them, which is what avoids the gcc-UBSAN additional issue. The CI-run varPro tests cover regression + classification only (no survival), so issue #118 cannot flake CI. Verified locally under NOT_CRAN=true: 0 failures across the varPro suite. NEWS.md / cran-comments.md restore the accurate "runs in CI and locally; skipped only on CRAN" claim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): v3.1.2 — skip only isopro(method="unsupv"), the sole gcc-UBSAN trigger (#122) * fix(cran): v3.1.2 — skip_on_cran the isopro fixture (clear gcc-UBSAN) v3.1.1 guarded the varPro forest-growing test fixtures with skip_on_cran() but missed make_iso_fit() in tests/testthat/test_gg_isopro.R. It was gated only by skip_if_not_installed("varPro"), and varPro IS installed on CRAN's check machines — so the ~23 isopro tests still ran there. varPro::isopro() grows an *unsupervised* isolation forest (do.call("rfsrc", ...) with the unsupv family → yvar.wt length 0). That is the exact path that trips randomForestSRC's gcc-UBSAN report at entry.c:184 (unconditional RF_yWeight-- on a 0-length weight vector → out-of-bounds pointer, UB). It was the one unsupervised grow v3.1.1 left unguarded, so the additional check re-flagged the issue against 3.1.1. Fix: add testthat::skip_on_cran() to make_iso_fit(), matching the other varPro fixtures. ggRandomForests is pure R (NeedsCompilation: no); package code is unchanged. The isopro tests still run in CI and locally (NOT_CRAN=true). Verified: R CMD check --as-cran (with manual) → 0 errors, 0 warnings, 1 NOTE (days-since-update), 2.6 min. test-all.Rout shows all 23 isopro tests in the "On CRAN" skip list, so the unsupervised grow no longer executes under the check. Bumps DESCRIPTION + NEWS to 3.1.2; cran-comments updated. Upstream fix is randomForestSRC commit 92ec283 (not yet on CRAN). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cran): narrow the UBSAN skip to only isopro(method="unsupv") Empirically pinned the gcc-UBSAN trigger with -fsanitize=undefined on CRAN's randomForestSRC 3.6.2: of every varPro/rfsrc grow in the suite, ONLY varPro::isopro(method="unsupv") fires entry.c:184. It is the sole grow with a length-0 yvar.wt (unsupervised family). Everything else is supervised or synthetic-supervised and UBSAN-clean: - varpro / ivarpro / beta.varpro: real Y -> non-empty yvar.wt - isopro(method="rnd"): synthetic supervised forest - uvarpro: grows yxyz123 ~ . with an rnorm response (synthetic supervised) So: - make_iso_fit() now skip_on_cran() ONLY when method == "unsupv" (the rnd isopro tests run on CRAN again). - Reverted v3.1.1's blanket skip_on_cran() on the varpro/uvarpro/ivarpro/ beta fixtures (helper-varpro-fixtures.R, test_gg_varpro.R, test_gg_udependent.R) -> ~126 varPro tests run on CRAN again. Verified: - Full suite under CRAN conditions (NOT_CRAN unset) against a GCC-equivalent UBSAN rfsrc (-fsanitize=undefined -fno-sanitize=float-cast-overflow): 0 runtime errors, FAIL 0 / PASS 1093 / SKIP 7. Only 2 tests skip On CRAN (isopro-unsupv + the pre-existing slow ivarpro test). - R CMD check --as-cran (with manual): 0 errors, 0 warnings, 1 NOTE (days-since-update); overall 2.68 min, well under the 10-min budget. Note: a local clang UBSAN build also surfaces partial.c:92 (a (uint)NaN float-cast in test_gg_partial_varpro's categorical survival partial dependence). CRAN does not see it: GCC's -fsanitize=undefined excludes float-cast-overflow (clang's includes it). Confirmed by rebuilding with that check disabled. Separate latent upstream issue, not a CRAN blocker. NEWS + cran-comments updated to describe the narrowed fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(isopro): namespace-qualify skip_* in make_iso_fit Use testthat::skip_on_cran()/skip_if_not_installed() so the helper does not depend on testthat being attached. Addresses a Copilot review note on #122; qualifies both calls for internal consistency. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran-comments): correct NOTE disposition + narrowed-skip wording R CMD check --as-cran returns 1 NOTE (days since last update), not 0; only the single unsupervised isopro test skips on CRAN now (all other varPro tests run). Genericise the day count so the submitted comment is not stale. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore(cran): record 3.1.2 submission (#123) submit_cran() recorded the submitted commit (SHA 83d8021, the merged #122) and timestamp 2026-06-13. ggRandomForests 3.1.2 submitted to CRAN to clear the gcc-UBSAN additional issue. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * v3.2.0: RMST(τ) partial computation + #118 gg_varpro guard (#127) * fix(gg_partial_varpro): drive RMST(tau) partial computation, not just label (v3.2.0) varPro::partialpro() has no time argument, so its default survival learner returns ensemble mortality at every horizon. gg_partial_varpro(scale="rmst", time=tau) previously only relabeled the y-axis, so multi-horizon RMST plots differed by Monte-Carlo noise rather than tau. scale="rmst" now passes partialpro() an RMST(tau) learner that integrates the survival curve (integral_0^tau S(t) dt) from object$rf, so the curve genuinely depends on tau. This recomputes from object (a survival fit) with part_dta=NULL; a precomputed part_dta can only be relabeled, and the function now warns when you try. Also warns when tau exceeds the model's event-time range (RMST truncated there) and when time is passed to a scale that ignores it. New internal helpers .rmst_learner() and .rmst_from_survival(); RMST math unit tests run on CRAN, the heavy end-to-end varpro fit is skip_on_cran() to protect the check-time budget. Minor release 3.1.2 -> 3.2.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_varpro): clear error on degenerate varPro importance table (#118) gg_varpro() failed with the cryptic "arguments imply differing number of rows: <p>, 0" when varPro::importance() returns a degenerate importance table (0 rows, or p variables with no usable z column) -- observed intermittently on survival fits where the release-rule step selects no variables. .build_varpro_imp_dfs() now detects that and stops with a clear, specific message suggesting a larger ntree. Scoped to the degenerate case; well-formed fits (survival included) are unaffected -- not a blanket survival-family block (cf. the reverted #116). Cherry-picked from the dev-line fix in 21805e4 (the gg_varpro.R guard + its CRAN-safe unit test only, not the bundled uvarpro feature work). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * build: require varPro (>= 3.1.0) in Imports The RMST(tau) partial path passes partialpro() a `learner`, an argument present since varPro 3.1.0 (the current CRAN version). Pin the floor so the dependency is explicit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_partial_varpro): address Copilot review on RMST guardrails Two fixes from the PR #127 review: - The tau-range warning fired for tau < min(time.interest), but a small tau is valid: RMST integration assumes S(t)=1 on [0, times[1]), so it is not truncated. Only warn when tau exceeds the largest event time (the actual truncation case, since S(t) cannot be extrapolated past max(ti)); message corrected accordingly. - Validate that 'time' is a single finite numeric. It now drives the RMST integration (and the surv/chf snap) as a scalar; a vector would silently recycle and return incorrect results. Tests added for both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(gg_partial_varpro): cover RMST/scale internals; fix C-path model on empty frame Addresses the Codecov patch-coverage comment on PR #127 (was 84% patch / gg_partial_varpro.R -4.16%). Adds CRAN-safe tests for the RMST-path internals that were uncovered: .resolve_varpro_scale() auto-by-family branches, .rmst_from_survival() bare-vector input, the .rmst_learner() OOB + newdata branches (via a small rfsrc fit, no varpro grow), the plain partialpro(object) mortality recompute (folded into the skip_on_cran e2e), and the C-path model label. File coverage 91% -> 99%. The C-path model test surfaced a real bug: .gg_partial_varpro_cpath() assigned a scalar `model` column to pd$continuous/pd$categorical, which errors on a 0-row data.frame ("replacement has 1 row, data has 0 rows") when a variable yields an all-continuous or all-categorical split. Guarded with nrow > 0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(gg_partial_varpro): cut .validate_varpro_inputs cyclomatic complexity The RMST scalar-time and survival-fit checks pushed .validate_varpro_inputs to cyclomatic complexity 28, over the lintr cyclocomp_linter limit of 20 (CI lint failure on PR #127). Extract two helpers -- .validate_partial_time() and .validate_rmst_inputs() -- per the linter's own suggestion. Behavior and error messages unchanged; package lints clean, tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gg_partial_varpro): forward ... to partialpro() (variable selection) The object-driven path called partialpro(object) / partialpro(object, learner=...) with no xvar.names, so variable selection fell back entirely to varPro::get.topvars(object) -- which returns few or no variables for some fits, yielding empty continuous/categorical frames. Because the RMST path must recompute from `object` (it cannot accept a precomputed part_dta carrying the learner), callers had no way to specify variables the way an explicit partialpro(xvar.names=...) call would. Add `...` to gg_partial_varpro() (and the gg_partialpro() alias), forwarded to partialpro() on the recompute path, so xvar.names / nvar / cut / nsmp etc. work again. Warns when `...` is passed alongside a precomputed part_dta (ignored). Matches the ...-forwarding convention of the other varPro wrappers (gg_beta_varpro, gg_ivarpro). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_partial_varpro): align RMST integration to the prediction's own time grid The RMST learner integrated predict.rfsrc()$survival against the closure's rf$time.interest, but predict.rfsrc() can return survival on a different column grid (observed on a real survival fit: newdata predictions came back ~40x too small, e.g. RMST(365) = 5-9 vs the OOB 109-365 for the same subjects). The mismatch did not error -- R's negative indexing (surv[, -n_times]) silently misaligned the integral -- so partialpro built RMST partial curves on wrong values while still looking populated. Use each prediction's own $time.interest for the integration, and assert in .rmst_from_survival() that ncol(surv) == length(times) so any future grid mismatch fails loud instead of returning a wrong number. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran): refresh cran-comments.md for v3.2.0 submission Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(gg_partialpro): stop terse @param tags from overriding the shared Rd gg_partialpro shares @Rdname gg_partial_varpro and sorts after it in collation, so its "Passed to gg_partial_varpro" @param tags overrode the rich descriptions on the canonical gg_partial_varpro.Rd page -- every argument rendered as the circular "Passed to gg_partial_varpro", burying the real time/scale/... docs. Drop the duplicate @param block from the alias (it shares gg_partial_varpro's formals, so those definitions now populate the shared Rd). Addresses the Copilot review on PR #127. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran): note win-builder R-devel/R-release 0/0/0 in cran-comments Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran): win-builder R-oldrelease also 0/0/0 (all three legs clean) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(cran): mac-builder 0/0/0 — all external builders clean Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * chore(cran): record v3.2.0 submission (SHA 9892d5a) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #109. The merged v3.1.0 still ships a 17.1 MB install / 9.0 MB tarball because the survival + regression vignettes embed interactive
plotly3D surfaces — self-contained quarto inlines plotly.js (~3.5 MB) into each HTML — and figures render at retina 2×. That is well over CRAN's 5 MB guideline and plausibly contributed to the v3.0.0 auto-hold.Changes
plot_ly()3D partial-dependence surfaces with staticggplot2heat maps (geom_tile+ viridis). Same analysis, no interactivity.fig-format: png+fig-dpi: 96in all four vignettes (was retina 2× → 192 dpi).plotlyfromSuggests(no longer used anywhere).Result
Verification
R CMD check --as-cranwith manual build,ggraphpresent: Status: OK — 0/0/0 (installed-size now reports as INFO, not a NOTE). All four vignettes rebuild clean.Note: the win-builder/mac-builder gate should be re-run on this trimmed package before CRAN submission — the earlier external-builder attempts used the 9 MB tarball.
🤖 Generated with Claude Code