Skip to content

fix #118 + unsupervised varPro wrappers (gg_beta_uvarpro / gg_sdependent) [dev]#121

Merged
ehrlinger merged 15 commits into
devfrom
fix/118-gg-varpro-empty-importance
Jun 14, 2026
Merged

fix #118 + unsupervised varPro wrappers (gg_beta_uvarpro / gg_sdependent) [dev]#121
ehrlinger merged 15 commits into
devfrom
fix/118-gg-varpro-empty-importance

Conversation

@ehrlinger

@ehrlinger ehrlinger commented Jun 12, 2026

Copy link
Copy Markdown
Owner

Re-cut as 3.1.3. This content was the originally-planned 3.1.2 (fix #118 + the unsupervised varPro wrappers), but that version number was consumed by the gcc-UBSAN re-fix shipped as 3.1.2. Rebased onto current main and bumped to 3.1.3.

Contents

Fixes applied during the re-cut

  • pkgdown CI failure → added the 4 new exported topics to _pkgdown.yml's reference index (they were documented exports missing from the index; pkgdown aborts on that).
  • Merge with main → resolved DESCRIPTION/NEWS only (code didn't conflict with the 3.1.1/3.1.2 test-skip changes); notes re-homed under a v3.1.3 heading.
  • Folded in the vignette \VignetteIndexEntry fix ("Vignette's Title" → real title) — supersedes docs(vignette): fix placeholder VignetteIndexEntry #124 (close it).

Notes

  • New exported functions → if you'd prefer a minor bump (3.2.0) over 3.1.3 by semver, it's a 2-line change; say the word.
  • Watch the re-run CI: pkgdown should now pass; the full R CMD matrix passed before the rebase.

🤖 Generated with Claude Code

ehrlinger and others added 5 commits June 10, 2026 10:04
* 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>
…ze (#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>
…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>
* 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>
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>
@codecov

codecov Bot commented Jun 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.46377% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.90%. Comparing base (8ece071) to head (fef4bd5).

Files with missing lines Patch % Lines
R/gg_beta_uvarpro.R 95.41% 5 Missing ⚠️
R/gg_sdependent.R 98.73% 1 Missing ⚠️
R/plot.gg_beta_uvarpro.R 97.61% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #121      +/-   ##
==========================================
+ Coverage   87.26%   87.90%   +0.63%     
==========================================
  Files          48       52       +4     
  Lines        4090     4364     +274     
==========================================
+ Hits         3569     3836     +267     
- Misses        521      528       +7     
Files with missing lines Coverage Δ
R/gg_varpro.R 100.00% <100.00%> (ø)
R/plot.gg_sdependent.R 100.00% <100.00%> (ø)
R/gg_sdependent.R 98.73% <98.73%> (ø)
R/plot.gg_beta_uvarpro.R 97.61% <97.61%> (ø)
R/gg_beta_uvarpro.R 95.41% <95.41%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ehrlinger ehrlinger changed the title fix(#118): clear error when varPro importance() returns empty (v3.1.2) Release 3.1.2: #118 gg_varpro fix + unsupervised varPro wrappers Jun 12, 2026
ehrlinger and others added 3 commits June 12, 2026 10:49
…p + 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>
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 named 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 (length(z) !=
length(vars), or 0 vars) and stops with a clear, specific message
suggesting a larger ntree.

Scoped to the degenerate case only: well-formed fits (survival included)
have length(z) == length(vars) > 0 and are unaffected. This is NOT a
blanket survival-family block (cf. the reverted #116).

Tests exercise the guard via the internal with a constructed importance
table (CRAN-safe, no varPro grow), including a positive case proving the
working path is preserved. Version bump to 3.1.2 (DESCRIPTION + NEWS).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Fold the unsupervised varPro wrappers (originally PR #120 on dev) into the
3.1.2 release line alongside the #118 fix:

- gg_beta_uvarpro() / plot/print/summary/autoplot: tidy wrapper for
  varPro::get.beta.entropy() (unsupervised lasso importance), the analogue
  of gg_beta_varpro(); colMeans(|beta|) per variable + cutoff/selected, with
  a precomputed beta_fit cache path.
- gg_sdependent() / plot/print/summary/autoplot: tidy wrapper for
  varPro::sdependent() signal-variable detection (imp_score / degree /
  signal flag), complementing gg_udependent()'s graph. Frame-building split
  into .gg_sdependent_build() to keep cyclomatic complexity under the lint
  cap.

Tests are CRAN-safe (constructed matrices, no varPro grow) with skip_on_cran
live-integration cases. lint_package() = 0; NEWS entries under v3.1.2.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehrlinger ehrlinger force-pushed the fix/118-gg-varpro-empty-importance branch from 51e9c0b to 4d23f4f Compare June 12, 2026 16:37
@ehrlinger ehrlinger changed the base branch from fix/cran-ubsan-varpro-skip to main June 12, 2026 16:37
ehrlinger and others added 2 commits June 13, 2026 09:41
…BSAN 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>
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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prepares ggRandomForests 3.1.2 by (1) hardening gg_varpro() against degenerate/empty varPro::importance() output (issue #118) and (2) adding new unsupervised varPro (uvarpro) wrappers for lasso-beta importance and signal-variable detection, with companion plot/print/summary/autoplot support and tests.

Changes:

  • Add a scoped guard in .build_varpro_imp_dfs() to stop with a clear error when importance() returns an unusable table (0 rows or missing/empty z), addressing #118.
  • Introduce gg_beta_uvarpro() and gg_sdependent() wrappers (plus plot/print/summary/autoplot methods) and add CRAN-safe tests.
  • Bump package version/docs to 3.1.2 (DESCRIPTION, NEWS, NAMESPACE, Rd updates).

Reviewed changes

Copilot reviewed 11 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/testthat/test_gg_varpro_empty_importance.R Adds regression tests for the #118 degenerate-importance guard without requiring a live varPro grow.
tests/testthat/test_gg_sdependent.R Adds matrix-driven and integration tests for gg_sdependent() and companions.
tests/testthat/test_gg_beta_uvarpro.R Adds deterministic and integration tests for gg_beta_uvarpro() and companions.
R/plot.gg_sdependent.R New plot method for gg_sdependent (lollipop chart).
R/plot.gg_beta_uvarpro.R New plot method for gg_beta_uvarpro (bar chart + cutoff line).
R/gg_varpro.R Adds guard to prevent cryptic failures when importance() output is degenerate.
R/gg_sdependent.R New gg_sdependent() wrapper + print/summary/autoplot + empty handling.
R/gg_beta_uvarpro.R New gg_beta_uvarpro() wrapper + validation + print/summary/autoplot + empty handling.
NEWS.md Adds 3.1.2 release notes covering #118 fix and new wrappers.
NAMESPACE Registers/exports new S3 methods and functions; imports geom_segment.
man/summary.gg.Rd Updates summary method documentation to include new summary methods.
man/print.gg.Rd Updates print method documentation to include new print methods.
man/plot.gg_sdependent.Rd New Rd for plot.gg_sdependent.
man/plot.gg_beta_uvarpro.Rd New Rd for plot.gg_beta_uvarpro.
man/gg_sdependent.Rd New Rd for gg_sdependent.
man/gg_beta_uvarpro.Rd New Rd for gg_beta_uvarpro.
DESCRIPTION Bumps version to 3.1.2.
Files not reviewed (6)
  • man/gg_beta_uvarpro.Rd: Generated file
  • man/gg_sdependent.Rd: Generated file
  • man/plot.gg_beta_uvarpro.Rd: Generated file
  • man/plot.gg_sdependent.Rd: Generated file
  • man/print.gg.Rd: Generated file
  • man/summary.gg.Rd: Generated file

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread R/gg_beta_uvarpro.R
Comment thread R/gg_sdependent.R Outdated
The originally-planned 3.1.2 content (fix #118 + the gg_beta_uvarpro/
gg_sdependent unsupervised varPro wrappers) lost its version number to the
gcc-UBSAN re-fix that shipped as 3.1.2. Re-cut here as 3.1.3.

- Merge main (v3.1.2): DESCRIPTION/NEWS reconciled to 3.1.3; #121's notes
  re-homed under a v3.1.3 heading above main's v3.1.2 (UBSAN) section.
- Fix the failing pkgdown build: add the 4 new exported topics
  (gg_beta_uvarpro, gg_sdependent, + plot.* ) to _pkgdown.yml's reference
  index (they were documented exports missing from the index).
- Fold in the vignette \VignetteIndexEntry fix (was the template placeholder
  'Vignette's Title') so 3.1.3 ships it; supersedes PR #124.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehrlinger ehrlinger changed the title Release 3.1.2: #118 gg_varpro fix + unsupervised varPro wrappers Release 3.1.3: #118 gg_varpro fix + unsupervised varPro wrappers Jun 13, 2026
@ehrlinger ehrlinger requested a review from Copilot June 13, 2026 19:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 38 out of 120 changed files in this pull request and generated 5 comments.

Files not reviewed (6)
  • man/gg_beta_uvarpro.Rd: Generated file
  • man/gg_sdependent.Rd: Generated file
  • man/plot.gg_beta_uvarpro.Rd: Generated file
  • man/plot.gg_sdependent.Rd: Generated file
  • man/print.gg.Rd: Generated file
  • man/summary.gg.Rd: Generated file

Comment thread R/gg_beta_uvarpro.R
Comment thread vignettes/.gitignore
Comment thread .superpowers/brainstorm/40719-1779304145/state/server.log Outdated
Comment thread .superpowers/brainstorm/37674-1779299963/state/server.log Outdated
Comment thread vignettes/ggRandomForests-regression_files/libs/quarto-html/tabsets/tabsets.js Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 38 out of 120 changed files in this pull request and generated 7 comments.

Files not reviewed (6)
  • man/gg_beta_uvarpro.Rd: Generated file
  • man/gg_sdependent.Rd: Generated file
  • man/plot.gg_beta_uvarpro.Rd: Generated file
  • man/plot.gg_sdependent.Rd: Generated file
  • man/print.gg.Rd: Generated file
  • man/summary.gg.Rd: Generated file

Comment thread R/gg_beta_uvarpro.R
Comment thread R/gg_sdependent.R
Comment thread R/gg_beta_uvarpro.R
Comment thread vignettes/.gitignore
Comment thread vignettes/ggRandomForests-survival_files/libs/quarto-html/tabsets/tabsets.js Outdated
Comment thread vignettes/ggRandomForests-regression_files/libs/quarto-html/tabsets/tabsets.js Outdated
Comment thread .superpowers/brainstorm/40719-1779304145/state/server.log Outdated
Addresses Copilot review on #121:
- gg_beta_uvarpro(): validate that `cutoff` is a single non-NA numeric (or
  NULL) so a vector can't be silently recycled into the `selected` comparison.
- gg_sdependent(): validate `threshold` (single numeric) and `directed`
  (single TRUE/FALSE) up front instead of failing cryptically downstream.
- .validate_beta_uvarpro(): take a `caller` arg so a bad `beta_fit` passed to
  gg_sdependent() reports 'gg_sdependent:' rather than 'gg_beta_uvarpro:'.
- Untrack and gitignore .superpowers/ (local brainstorm state, leaks abs paths)
  and vignettes/*_files/ + rendered *.html (regenerated build artifacts,
  already .Rbuildignore'd). Kept on disk; removed from the repo.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehrlinger

Copy link
Copy Markdown
Owner Author

Addressed the review items in 7084f624:

  • cutoff validation (gg_beta_uvarpro): now errors unless cutoff is a single non-NA numeric (or NULL) — no more silent vector recycling into selected.
  • gg_sdependent input validation: threshold (single numeric) and directed (single TRUE/FALSE) validated up front.
  • .validate_beta_uvarpro(caller=): a bad beta_fit passed to gg_sdependent() now reports gg_sdependent:, not gg_beta_uvarpro:.
  • Repo hygiene: untracked + gitignored .superpowers/ (local brainstorm state) and vignettes/*_files/ + rendered *.html (regenerated build artifacts, already .Rbuildignore'd). Kept on disk; removed from the repo (39k lines of generated assets dropped).

3.1.2 just shipped to CRAN; the #118 fix + new unsupervised varPro wrappers
are normal development, not a back-to-back CRAN update. Drop the 3.1.3 version
bump back to the dev version (3.1.2.9000) and file the notes under the
v4.0.0 (development) line. To be retargeted onto dev and rebased after the
forward-merge (#126) lands.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehrlinger ehrlinger changed the title Release 3.1.3: #118 gg_varpro fix + unsupervised varPro wrappers fix #118 + unsupervised varPro wrappers (gg_beta_uvarpro / gg_sdependent) [dev] Jun 13, 2026
@ehrlinger ehrlinger changed the base branch from main to dev June 13, 2026 20:46
@ehrlinger ehrlinger marked this pull request as draft June 13, 2026 20:46
ehrlinger and others added 2 commits June 13, 2026 18:56
Now that the forward-merge (#126) landed on dev, bring dev into this branch so
the diff is just the new work (#118 fix + gg_beta_uvarpro / gg_sdependent).

Resolutions:
- test_gg_vimp.R, precompute_varpro.R, R/print_helpers.R: took dev (the
  deterministic gg_vimp fix, the MASS guard, the RHF provenance block; this
  branch made no competing changes to those).
- NEWS.md: combined both v4.0.0 (development) sections (RHF + the new wrappers).
- NAMESPACE + man/: regenerated via roxygen so both the RHF and the new
  unsupervised-varPro exports are present.

Stays a draft: development work at 3.1.2.9000, not a release.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The input-validation added for the Copilot review pushed gg_beta_uvarpro.uvarpro
(25) and gg_sdependent.uvarpro (28) over the package's cyclomatic-complexity
limit (20). Extract the checks into helpers:
- gg_sdependent: reuse the existing .validate_udep_inputs(object, threshold,
  directed) (the centralized check gg_udependent uses — what Copilot suggested).
- gg_beta_uvarpro: .assert_scalar_numeric_or_null() for cutoff.
- shared .is_empty_beta_matrix() for the empty-matrix fast-path in both.

cyclocomp now ≤20 (lint_package() = 0 lints); test_gg_beta_uvarpro (43) +
test_gg_sdependent (27) pass. Validation behaviour unchanged except threshold
is now required positive (matching gg_udependent's contract).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@ehrlinger ehrlinger marked this pull request as ready for review June 13, 2026 23:46
@ehrlinger ehrlinger merged commit 21805e4 into dev Jun 14, 2026
9 checks passed
@ehrlinger ehrlinger deleted the fix/118-gg-varpro-empty-importance branch June 14, 2026 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

gg_varpro(): intermittent failure on survival fits — varPro importance() returns 0 rows

2 participants