Merge main (v3.4.0/3.4.1) into dev_rhf#140
Open
ehrlinger wants to merge 23 commits into
Open
Conversation
* 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>
…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>
…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>
* 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>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rv defaults) (#129) * docs: open v3.3.0; fold in the RMST "Reading an RMST curve" interpretation section Starts the 3.3.0 minor (off main) and pulls in the RMST-interpretation @section that was prepared on docs/3.2.1-rmst-interpretation, re-versioned 3.2.1 -> 3.3.0. The standalone 3.2.1 doc release is superseded by this cycle (the upcoming varPro classification probability-scale figure work lands here too). Also gitignore brainstorm/vignette/tarball build artifacts that were previously untracked. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): v3.3.0 design — interpretable y-axis scales for varPro partial plots Classification partial plots default to probability (odds/logodds options); survival gains an S(t) learner (scale='surv', user-supplied tau) and no longer silently defaults to ensemble mortality. Causal contrast hidden on bounded scales. Approved design, pre-implementation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): survival default tau — median follow-up, not an error (3.3.0) Revise 3e: scale='auto'+survival now returns survival probability S(tau) at a data-driven default tau (median follow-up), units-safe by construction; surv and rmst both default tau when time is omitted. Replaces the earlier 'auto errors' design. Mortality stays an explicit opt-in. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(spec): require explicit causal-curve documentation (what it is + when to use) §4: dedicated section explaining causal = varPro's baseline-subtracted virtual-twins estimator (Ishwaran & Blackstone 2025), when to use it (effect vs level; cross-check), the not-a-structural-claim caveat, and why it's hidden on bounded scales. Add the virtual-twins reference. §3d links to it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(plan): v3.3.0 varPro partial-plot scales — TDD implementation plan 10 tasks, TDD per step: scale vocab/resolution, prob/odds conversion (mean of probabilities), causal blanking, S(t) learner, median-follow-up default tau, routing/validation, target-class labels, plot causal warning, docs/NEWS, full gate. Grounded in current internals. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gg_partial_varpro): probability/odds/surv scales + S(t) learner + default tau (3.3.0) Classification partial plots default to probability (scale auto->prob; odds/ logodds options; mean-of-probabilities back-transform). Survival defaults to S(tau) via a new partialpro learner (auto->surv); surv/rmst default tau to median follow-up (units-safe). Causal contrast blanked + dropped/warned on bounded scales. Target-class provenance + new y-labels. Reconciled C-path tests to chf; full file green (122). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(gg_partial_varpro): scale @details, prob/surv/causal @Sections, NEWS, virtual-twins ref Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(3.3.0): gg_partialpro scale vocabulary + escape [0,1] in roxygen - gg_partialpro() alias scale default now matches gg_partial_varpro's 8-value vocabulary (was the old 5-value vector, breaking match.arg when forwarded). - \eqn{[0, 1]} in two @details/@section blocks: markdown mode had turned the bare [0, 1] into a \link, tripping the Rd cross-reference check. R CMD check --as-cran now clean (only the days-since-update + local HTML-tidy NOTEs remain, both environmental). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(test): split compound skip_*() semicolons (lint) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_partial_varpro): scale-aware messages for surv/rmst (Copilot review) .validate_rmst_inputs renamed .validate_surv_scale_inputs and its stop message now names the actual scale (was hard-coded 'rmst', misleading for surv). .warn_varpro_rmst's precomputed-part_dta and tau-out-of-range warnings are now scale-generic (were RMST-specific, misleading for surv). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(vignette): update varpro partial-dependence sections for 3.3.0 scales Classification section now describes the probability default (P(Y=target), causal hidden, odds/logodds options); survival section rewritten from the stale 'C-path / ensemble mortality' prose to the S(tau) survival-probability default via the partialpro learner, the units-safe median-follow-up tau, and mortality/rmst/chf as explicit opt-ins. Fixed the family-support table cell and two stale mortality/C-path mentions. Re-ran precompute so the bundled .rds caches pd_iris_multi (prob) and pd_pbc (surv, tau=median follow-up); vignette still loads from cache (no live grow, check-time unchanged). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_partial_varpro): default tau only when recomputing; fix 'one-time' vignette wording (Copilot) - The median-follow-up default tau now resolves only when part_dta is NULL (recomputing from object). A precomputed part_dta is label-only, so the old code emitted a misleading 'default horizon' message and could call .default_surv_tau() on a non-survival rf. Extracted to .resolve_default_tau() (also keeps gg_partial_varpro under the cyclocomp limit). Regression test added. - Vignette: the tau message fires on every omitted-time call, not once; dropped the inaccurate 'one-time' wording. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
… V3 CRAN line (#130) * feat: port gg_beta_uvarpro + gg_sdependent to the V3 CRAN line (3.4.0) Brings the two uvarpro visualization wrappers (varPro::get.beta.entropy bar chart; varPro::sdependent signal-variable lollipop) from the dev_rhf line down to a V3 3.4.0 minor, per the V3=varPro / V4=randomForestRHF boundary. New files only; no new dependency (varPro already in Imports). Tests carry skip_on_cran for the uvarpro-grow gcc-UBSAN issue. Version 3.3.0 -> 3.4.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(vignette): add gg_beta_uvarpro + gg_sdependent to the uvarpro section (3.4.0) Two subsections after gg_udependent, run live on the cached u_boston fit (consistent with how gg_udependent is shown): the entropy-ranking bar chart and the signal-variable lollipop. No precompute change (both are post-fit processors on the existing uvarpro fit, like get.beta.entropy/sdependent already used by gg_udependent in the vignette). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* docs(voice): voice-polish pass on the 3.3.0/3.4.0 prose (3.4.0) Applies the writing-voice harness (persona: biostatistician/data-scientist) to the spots a documentation scan flagged -- mostly the new 3.3.0/3.4.0 prose: - RMST @section: drop the 'two consequences worth stating' meta-framing and the em-dash clusters; 'Two things follow. First... Second...'. - survival vignette: 'unbounded relative-risk score, not a probability, works well as a single scalar' sterile balance -> plain 'not a probability; a single risk score for ranking'; vary the boilerplate tricolon (was verbatim-identical to the regression vignette). - regression vignette: 'stable, low-variance estimator' -> 'smooth out the noise'. - varpro vignette: gg_sdependent section now opens from the reader ('you have a ranking; now you want the cut line'), not the function; gg_varpro reading guide primed before the plot. - README: authorless data blurb gets 'you'; drop the duplicated ggplot/patchwork gloss + tricolon (the 'Why?' section already explains it). Prose only -- no code, no behavior, no version change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(voice): fix grammar/accuracy from voice pass (Copilot review) - varpro vignette: 'a tight box is a variable...' -> 'a tight box means every tree agrees on that variable'. - README: restore the ggplot/patchwork gloss on the plot() line -- 'building on with +' was inaccurate for patchwork composites. - RMST @section: 'buy more' -> 'buy you more' (missing object); Rd regenerated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…ntry (#132) * docs: add short uvarpro vignette; trim varpro; fix stale VignetteIndexEntry Add a focused vignette for the unsupervised varPro wrappers and lift the three unsupervised sections out of the (now supervised-only) varPro vignette so there is one source of truth. Also fix a stale CRAN listing. * New vignettes/uvarpro.qmd: walks one mtcars uvarpro() fit through gg_udependent() (structure), gg_beta_uvarpro() (ranking), and gg_sdependent() (the signal/noise cut), using the shared beta_fit matrix. Renders clean (3 figures incl. the ggraph network; citation resolves). * varpro.qmd: reworked intro from "six wrappers" to "five supervised wrappers" with a pointer to the new vignette; replaced the three worked uvarpro subsections with a short pointer; fixed the broken cross-reference; removed the gg_udependent row from the family-support matrix. * precompute_varpro.R: removed the now-dead u_boston fit, its list entry, and the stale comment. * ggRandomForests.qmd: fixed \VignetteIndexEntry, which still carried the quarto template placeholder "Vignette's Title" (what CRAN listed) -- now "Exploring Random Forests with ggRandomForests". * .gitignore: ignore the uvarpro_* build artifacts (.Rbuildignore already covers them via wildcards). No version bump: this folds into the accumulating 3.4.0 delta. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: register uvarpro in pkgdown index; fix fragile §6 cross-refs * _pkgdown.yml: add the new uvarpro vignette to both the top-level articles index (the missing entry that failed the pkgdown build: "1 vignette missing from index: uvarpro") and the navbar Articles menu. * varpro.qmd: replace two hard-coded "§6" section references with relative wording ("the closing reference section", "the end of this vignette"). The vignette doesn't number sections, so §6 resolved to nothing (Copilot review). Also dropped a stale "deferred to v3.1.0". Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
#133) * fix(gg_partial_rfsrc): impose factor levels by integer code, not label partial.rfsrc() imposes a factor level by its internal integer code -- it runs as.numeric(partial.values) internally. gg_partial_rfsrc() was passing the level *labels*, so character labels ("No"/"Yes") coerced to NA and numeric-looking labels ("4"/"6"/"8") to out-of-range codes, and every factor level collapsed to a single partial-dependence value (a flat categorical partial plot; "NAs introduced by coercion" warnings). Confirmed against ground truth (impose level on all obs, predict, mean) and plot.variable(partial=TRUE): passing integer codes reproduces both exactly; passing labels collapses all levels. Fix: make_eval_grid() passes seq-of-level codes for factors and carries the level vector; partial_one_var() relabels get.partial.plot.data()'s numeric x back to labels. Continuous and numeric low-cardinality predictors are unchanged. The stale comment claiming labels were required is corrected. Adds tests/testthat/test_gg_partial_rfsrc.R: factor levels must not collapse, ordering is recovered, and per-level means track ground truth. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(gg_partial_rfsrc): return categorical x as factor in model level order Addresses Copilot review on #133. Relabeling get.partial.plot.data()'s numeric x to a plain character vector let the downstream factor(.data$x) in plot.gg_partial_rfsrc() re-sort levels alphabetically (e.g. lo/mid/hi -> hi/lo/mid), losing the model's level order. split_partial_result() now returns categorical x as a factor with levels in first-appearance order. The rows arrive blocked by level in ascending code order (the model's level order), so this preserves it; factor(x) in the plot method then keeps those levels. Done after the mixed continuous/categorical rbind (x still character there) to avoid coercing the numeric continuous x. @return doc and a level-order test added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: rename single-letter L to lev (object_name_linter) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
#134) Release gate passed on merged main (incl. #132 docs + #133 factor-partial fix): R CMD check --as-cran WITH manual = Status OK (0/0/0), tarball 4.2 MB, total check 3m19s (vignette rebuild 36s), urlchecker 17/17, 0 revdeps. * cran-comments.md: rewrite for v3.4.0 (was stale at v3.2.0) -- interpretable varPro partial scales, unsupervised wrappers, the gg_partial_rfsrc factor fix; test envs and UBSAN NOTE disposition refreshed. * DESCRIPTION: Date -> 2026-07-01. * README.md docs sweep: add a pointer to the new uvarpro vignette (the unsupervised trio was undiscoverable from the README); refresh the stale "Recent changes" highlights (were stuck at v2.5/v2.6) to the v3.x line. Version stays 3.4.0 (DESCRIPTION == NEWS; carries the 3.3.0+3.4.0 delta). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
R CMD check flagged a non-standard top-level file `Rplots.pdf`. It's a default-device artefact created when a plot is drawn with no open device (interactive smoke-tests, testthat runs). It's already gitignored but was not build-ignored, so R CMD build copied it into the tarball. Add an unanchored `Rplots\.pdf$` rule so the artefact is excluded from the tarball at the root or under tests/ regardless of who left one behind. Verified: with strays present at both locations, the built tarball contains no Rplots.pdf. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…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>
A local CLAUDE.md (repo-specific Claude instructions, references the Obsidian codemap) sits at the repo root. It's not tracked, but nothing kept R CMD build from sweeping it into the tarball -> a "non-standard file at top level" NOTE, same class as the stray Rplots.pdf handled in #135. Add `^CLAUDE\.md$` to .Rbuildignore (never ship it) and `CLAUDE.md` to .gitignore (never commit it). Verified: with CLAUDE.md present in the source tree, the built tarball contains no CLAUDE.md. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…wn) (#138) The mac.r-project.org macOS builder is returning HTTP 502 at submission time, so no mac-builder result exists. Replace the (now-untrue) "mac-builder Status: OK" line with the honest statement: macOS is covered by the local aarch64-apple-darwin23 --as-cran check and the macos-latest GitHub Actions job. Also pin the win-builder R versions to the ones actually run (R-devel r90199 / release 4.6.1 / oldrel 4.5.3), all Status: OK. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…3.4.1) (#139) * fix: add default S3 methods to the classic rfsrc/randomForest wrappers (3.4.1) Post-release consistency pass. gg_error(), gg_vimp(), gg_variable(), gg_rfsrc(), and gg_brier() had no default S3 method, so a wrong-class input got R's generic "no applicable method" error. Add default methods emitting a clear "expected an 'rfsrc' or 'randomForest' object" message (naming the class received), matching the varPro-family cleanup done in 3.4.0. gg_roc() is intentionally left as-is: its default is aliased to gg_roc.rfsrc (accepts rfsrc-shaped objects); changing that is a behavior change, not a mechanical add. Additive only — no change to valid-input behavior. New fit-free dispatch test (runs on CRAN). Patch bump to 3.4.1 (DESCRIPTION + NEWS); will not be submitted immediately (CRAN discourages back-to-back updates) — it starts the post-3.4.0 dev line on main. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(cran): record 3.4.0 submission (SHA 16c8b4d) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Forward-merge the V3 CRAN line into the V4 (randomForestRHF) dev line so dev_rhf carries main's accumulated fixes: interpretable varPro partial scales (3.3.0), the gg_partial_rfsrc factor-code fix, the full dispatch cleanup (varPro family + classic wrappers .default), the uvarpro vignette + varpro.qmd trim, and the Rplots/CLAUDE.md build-ignore hygiene. Conflict resolutions: - DESCRIPTION: keep dev_rhf Version 4.0.0.9000; Date -> 2026-07-02. Both varPro (>= 3.1.0) and randomForestRHF (Suggests) retained. - NEWS.md: dev_rhf's v4.0.0 (development) heading on top, then main's v3.4.1 / v3.4.0 / v3.3.0 sections; single Version: line (4.0.0.9000). - NAMESPACE / man/: regenerated via document() -> union of main's .default methods (12 dispatch entries) and dev_rhf's RHF exports (gg_rhf/gg_auct). - R/gg_partial_varpro.R, R/gg_beta_uvarpro.R: took main (0 RHF refs; 3.3.0 scale work + reviewed uvarpro). - R/print_helpers.R: kept dev_rhf's RHF provenance branch. - test_gg_partial_varpro.R: main (3.3.0 scale tests). - test_gg_vimp.R: kept dev_rhf's deterministic version (importance[1] <- -1). - vignettes: took main's trimmed varpro.qmd + new uvarpro.qmd + precomputed rds (dropped u_boston); kept dev_rhf's MASS guard in precompute_varpro.R. - .Rbuildignore / .gitignore / cran-comments.md / CRAN-SUBMISSION / survival.qmd: took main (functional superset / current truth). Verified: package loads; test_default_dispatch 6/0, test_gg_vimp 63/0, test_gg_partial_varpro 108/0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev_rhf #140 +/- ##
==========================================
Coverage ? 88.44%
==========================================
Files ? 52
Lines ? 4526
Branches ? 0
==========================================
Hits ? 4003
Misses ? 523
Partials ? 0
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Forward-merge of the CRAN v3.4.0/3.4.1 line from main into the v4 development line (dev_rhf), keeping dev_rhf up to date with mainline fixes while preserving RHF-specific v4 work.
Changes:
- Adds/lands v3.3.0 varPro partial-plot scale updates (classification defaults to probability; survival defaults to survival probability at a data-driven horizon; bounded-scale plotting behavior/docs).
- Fixes
gg_partial_rfsrc()factor partial dependence by passing integer level codes and relabeling output; adds regression tests. - Improves S3 dispatch consistency by adding
.defaultmethods with clearer wrong-class errors; updates vignettes/NEWS/README/pkgdown and build-ignore hygiene.
Reviewed changes
Copilot reviewed 34 out of 39 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| vignettes/varpro.qmd | Updates supervised varPro vignette; points unsupervised material to new vignette and documents new partial scales. |
| vignettes/uvarpro.qmd | Adds a new vignette for unsupervised varPro wrappers (uvarpro() workflow). |
| vignettes/precompute_varpro.R | Trims precomputed artifacts (drops u_boston) to reduce cached RDS size. |
| vignettes/ggRandomForests-survival.qmd | Tightens/clarifies survival vignette prose around mortality interpretation. |
| vignettes/ggRandomForests-regression.qmd | Minor wording update in regression vignette intro. |
| tests/testthat/test_gg_partial_varpro.R | Expands coverage for new scale resolution/transform behavior and survival defaults/learners. |
| tests/testthat/test_gg_partial_rfsrc.R | New tests guarding correct factor-level handling in gg_partial_rfsrc(). |
| tests/testthat/test_gg_ivarpro.R | Adds dispatch/wrong-class error coverage via default method. |
| tests/testthat/test_gg_isopro.R | Adds dispatch/wrong-class error coverage via default method. |
| tests/testthat/test_gg_beta_varpro.R | Adds dispatch/wrong-class error coverage via default method. |
| tests/testthat/test_default_dispatch.R | New tests ensuring rfsrc/randomForest wrappers fail with clear default-method errors. |
| README.md | Mentions the new uvarpro vignette; refreshes wording and highlights. |
| R/plot.gg_partial_varpro.R | Updates plot behavior/docs: bounded scales drop causal, improved y-labels, and additional interpretation sections. |
| R/gg_vimp.R | Adds gg_vimp.default with a clearer wrong-class message. |
| R/gg_variable.R | Adds gg_variable.default with a clearer wrong-class message. |
| R/gg_rfsrc.R | Adds gg_rfsrc.default with a clearer wrong-class message. |
| R/gg_partialpro.R | Expands scale argument vocabulary to include prob/odds/logodds. |
| R/gg_partial_varpro.R | Implements scale resolution/conversion, survival S(τ) learner, default τ selection, provenance enhancements. |
| R/gg_partial_rfsrc.R | Fixes factor partial dependence by passing integer codes and mapping results back to labels; preserves model level order. |
| R/gg_ivarpro.R | Moves wrong-class check into gg_ivarpro.default (consistent S3 dispatch). |
| R/gg_isopro.R | Adds gg_isopro.default and removes unreachable inner class check. |
| R/gg_error.R | Adds gg_error.default with a clearer wrong-class message. |
| R/gg_brier.R | Adds gg_brier.default with a clearer wrong-class message. |
| R/gg_beta_varpro.R | Adds gg_beta_varpro.default and removes unreachable inner class check. |
| R/gg_beta_uvarpro.R | Removes redundant inherits() check in the class method (default method covers wrong-class). |
| NEWS.md | Merges/organizes v4 development notes plus v3.4.1/3.4.0/3.3.0 changelog entries. |
| NAMESPACE | Registers new S3 default methods and regenerated exports. |
| man/plot.gg_partial_varpro.Rd | Regenerated docs reflecting new plot interpretation sections and reference. |
| man/gg_partial_varpro.Rd | Regenerated docs reflecting new scale vocabulary/details and references. |
| man/gg_partial_rfsrc.Rd | Regenerated docs noting categorical x is now a factor preserving model level order. |
| dev/plans/2026-06-23-varpro-partial-scales-plan.md | Adds implementation plan document for v3.3.0 partial scale work. |
| dev/plans/2026-06-23-varpro-partial-scales-design.md | Adds design doc for v3.3.0 partial scale work. |
| DESCRIPTION | Updates Date to 2026-07-02 (v4 dev version retained). |
| CRAN-SUBMISSION | Updates submission metadata from mainline. |
| cran-comments.md | Updates CRAN comments to v3.4.0 consolidated submission notes. |
| .Rbuildignore | Ensures CLAUDE.md and plot artifacts are excluded from builds. |
| .gitignore | Ignores new vignette cache/artifacts and other local build outputs. |
| _pkgdown.yml | Adds uvarpro vignette to navbar/articles for pkgdown site. |
Files not reviewed (3)
- man/gg_partial_rfsrc.Rd: Generated file
- man/gg_partial_varpro.Rd: Generated file
- man/plot.gg_partial_varpro.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
585
to
589
| .process_cat_var <- function(feat, feat_name, scale = "generic") { | ||
| bounded <- .is_bounded_scale(scale) | ||
| n_cats <- length(unique(feat$xorg)) | ||
| cat_feat <- list() | ||
| for (ind in seq(n_cats)) { |
Owner
Author
There was a problem hiding this comment.
Fixed in fabcef3: .process_cat_var() now computes unique(feat$xorg) once into cats and builds plt.df with a single dplyr::bind_rows(cat_feat) at the end, dropping both O(k²) costs. Behavior unchanged — test_gg_partial_varpro still 108/0.
Copilot review (PR #140): .process_cat_var() recomputed unique(feat$xorg) every loop iteration and grew plt.df with an incremental bind_rows(), both O(k^2) in the number of categories. Compute the category labels once and bind the accumulated per-category frames in a single bind_rows() at the end. Behavior unchanged; test_gg_partial_varpro 108/0. Co-Authored-By: Claude Opus 4.8 <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.
Forward-merge the V3 CRAN line into the V4 (RHF) dev line
Keeps
dev_rhfcarryingmain's accumulated fixes so V4 doesn't regress on what V3 already fixed.mainwas 21 ahead,dev_rhf10 ahead — a genuine two-way divergence, so this was resolved deliberately (not an auto-merge).Brings onto dev_rhf: interpretable varPro partial scales (3.3.0), the
gg_partial_rfsrcfactor-code fix, the full dispatch cleanup (varPro family + classic wrappers.default), theuvarprovignette +varpro.qmdtrim, and theRplots.pdf/CLAUDE.mdbuild-ignore hygiene.Preserves dev_rhf's V4 work:
gg_rhf,gg_auct, RHF fixtures/snapshots/tests, theprint_helpersRHF branch, version4.0.0.9000, and thev4.0.0 (development)NEWS heading.Conflict resolutions (17 files)
4.0.0.9000, Date→2026-07-02; bothvarPro (>=3.1.0)+randomForestRHFretainedVersion:linedocument()→ union (12 RHF entries + 10.defaultmethods)importance[1]<--1version)Verification (local,
randomForestRHFinstalled)document()0 warnings.test_default_dispatch6/0,test_gg_vimp63/0,test_gg_partial_varpro108/0.test_gg_rhf19/0,test_gg_auct18/0,test_plot_gg_rhf9/0,test_plot_gg_auct7/0.Full commit-message log documents every per-file call. Please sanity-check the vignette/NEWS resolutions before merging.
🤖 Generated with Claude Code