docs: randomForest finishing work — design spec + implementation plans (PR #87 & #88)#87
Merged
Merged
Conversation
…) and PR #88 (per_class ROC)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #87 +/- ##
=======================================
Coverage 86.39% 86.39%
=======================================
Files 38 38
Lines 3021 3021
=======================================
Hits 2610 2610
Misses 411 411 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a design spec and two implementation plans for upcoming v2.8.0 “randomForest engine finishing work” (PRs #87 and #88), focusing on gg_variable.randomForest classification output shape and multi-class ROC support in gg_roc.
Changes:
- Add a cross-PR design spec describing intended behavior, tests, and files touched for #87 and #88.
- Add a step-by-step (TDD-oriented) implementation plan for PR #87 (
gg_variable.randomForestclassification + plot smooth fixes). - Add a step-by-step (TDD-oriented) implementation plan for PR #88 (multi-class
gg_rocviaper_class=TRUE+ plotting/summary updates).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| dev/plans/2026-05-21-rf-finishing-work-design.md | Design spec spanning PR #87/#88, including proposed API changes, behavior, and tests. |
| dev/plans/2026-05-21-rf-87-gg-variable-classification-plan.md | Implementation plan for gg_variable.randomForest classification fix + plot.gg_variable smooth fixes. |
| dev/plans/2026-05-21-rf-88-multiclass-roc-plan.md | Implementation plan for gg_roc(per_class=TRUE) multi-class OvR curves + plot/summary changes. |
Comments suppressed due to low confidence (1)
dev/plans/2026-05-21-rf-finishing-work-design.md:194
- In the PR #88 per_class section, the spec says the long-format data frame has columns
fpr/tpr, but the package’s ROC data contract issens/spec/pct(seeR/calc_roc.Rreturn docs andtests/testthat/test_gg_roc.R). Recommend specifying that per-class output retainssens/spec/pct(plusclass), and thatfpr = 1 - spec/tpr = sensare computed in the plot method if desired.
- For forests with > 2 classes: compute one-vs-rest ROC for every class *k*
(scores = `votes[, k]`, positive = `(y == k)`), stack into a long data frame
with columns `class` (factor), `fpr`, `tpr`.
- `attr(gg, "auc")` becomes a named numeric vector: one entry per class,
ordered by descending AUC. Class factor levels follow the same order.
- For binary forests: `per_class = TRUE` is a no-op — returns the single-curve
result with no `class` column and a scalar `auc` attribute (same as
`per_class = FALSE`).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **New signature:** | ||
|
|
||
| ```r | ||
| gg_roc(object, which_outcome = "all", per_class = FALSE, ...) |
Owner
Author
Comment on lines
+199
to
+201
| **Internal helper `calc_roc_one_vs_rest(scores, y, k)`** — single class OvR | ||
| ROC, returns a data frame `(fpr, tpr)` plus scalar AUC. Extracted so it can be | ||
| reused for both per-class computation and macro-average. |
Owner
Author
| gg_roc.rfsrc <- function(object, which_outcome, oob = TRUE, per_class = FALSE, ...) { | ||
| ``` | ||
|
|
||
| Note: the rfsrc per_class path is out of scope for this PR (tracked under issue #72). The argument is added only so `gg_roc(rfsrc_obj, per_class = TRUE)` does not error with "unused argument". Leave the body unchanged. |
Owner
Author
| rf <- randomForest::randomForest(Species ~ ., data = iris, ntree = 100L) | ||
| gg <- gg_roc(rf, per_class = TRUE) | ||
| expect_true("class" %in% names(gg)) | ||
| expect_true(all(c("sens", "spec") %in% names(gg))) |
Owner
Author
Comment on lines
+157
to
+164
| # For classification forests use per-class OOB vote fractions (object$votes), | ||
| # stored as yhat.<classname> columns — the same shape gg_variable.rfsrc | ||
| # produces. For regression a single numeric yhat column suffices. | ||
| if (object$type == "classification") { | ||
| preds <- object$votes # n × n_classes matrix of OOB vote fractions | ||
| colnames(preds) <- paste0("yhat.", colnames(preds)) | ||
| gg_dta <- cbind(gg_dta, preds) | ||
| gg_dta$yvar <- response |
Owner
Author
| rf <- randomForest::randomForest(Species ~ ., data = iris, ntree = 50L) | ||
| gg <- gg_variable(rf) | ||
| p <- plot(gg) | ||
| expect_true(inherits(p, "patchwork") || inherits(p, "ggplot")) |
Owner
Author
- design doc: add oob=TRUE to gg_roc new signature (was omitted) - design doc: replace proposed calc_roc_one_vs_rest() with existing .rf_prob_matrix/.rf_one_class_roc/.rf_macro_average_roc helpers; document that output uses sens/spec/pct (package contract), not fpr/tpr - plan #87: add norm.votes=FALSE row-normalization to votes extraction step - plan #87: tighten patchwork test expectation (iris 4-predictor default must return patchwork, not patchwork||ggplot, to catch list regressions) - plan #88: add pct to per_class column assertions (both occurrences) - plan #88: correct rfsrc per_class rationale (argument is for API discoverability, not to prevent "unused argument" error — rfsrc uses ...) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
…t review) Plan #87 was corrected (via PR #87 Copilot review) to assert patchwork specifically for the 4-predictor iris default-plot case, rather than the loose patchwork||ggplot form that wouldn't catch a regression to a bare list (issue #80). Align the implementation test to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
…tion (PR #87) Tests verify that gg_variable.randomForest classification forests produce: - Per-class vote fraction columns (yhat.setosa, yhat.versicolor, yhat.virginica) - No bare yhat column for multi-class prediction - Valid vote fractions (0-1) that row-sum to 1 - Plottable output for multi-xvar case - Layer_data access on single-xvar plots All three tests currently FAIL, as expected (TDD red phase). Implementation fix comes in T2.
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
#87) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
…t review) Plan #87 was corrected (via PR #87 Copilot review) to assert patchwork specifically for the 4-predictor iris default-plot case, rather than the loose patchwork||ggplot form that wouldn't catch a regression to a bare list (issue #80). Align the implementation test to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ehrlinger
added a commit
that referenced
this pull request
May 21, 2026
…h bugs (#87) (#88) * chore: open v2.7.3.9005 dev increment (PR #87) * test: add three failing tests for gg_variable.randomForest classification (PR #87) Tests verify that gg_variable.randomForest classification forests produce: - Per-class vote fraction columns (yhat.setosa, yhat.versicolor, yhat.virginica) - No bare yhat column for multi-class prediction - Valid vote fractions (0-1) that row-sum to 1 - Plottable output for multi-xvar case - Layer_data access on single-xvar plots All three tests currently FAIL, as expected (TDD red phase). Implementation fix comes in T2. * fix: gg_variable.randomForest classification uses object\$votes for yhat.* columns Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: normalise object\$votes rows to defend against norm.votes=FALSE; update stale oob comment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: add failing tests for plot.gg_variable smooth bugs (PR #87) * fix: plot.gg_variable binary smooth aes + add multi-class smooth block (#87) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: add vdiffr snapshots for gg_variable RF classification (PR #87) * docs: update NEWS for PR #87 — gg_variable RF classification fix * fix: plot.gg_variable multi-class facets use class names not integer indices Strip the yhat. prefix from column names when building the outcome column in the multi-class pivot loop (line ~169), so facet labels show "setosa"/ "versicolor"/"virginica" instead of 1/2/3. Add a regression test that verifies p@data$outcome contains class names after plotting. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test: tighten patchwork assertion to match corrected plan (#87 Copilot review) Plan #87 was corrected (via PR #87 Copilot review) to assert patchwork specifically for the 4-predictor iris default-plot case, rather than the loose patchwork||ggplot form that wouldn't catch a regression to a bare list (issue #80). Align the implementation test to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address Copilot review on PR #88 — oob warning + factor level order - gg_variable.randomForest: replace silent no-op oob override with an explicit warning when oob=FALSE is supplied, since in-bag class probabilities are not available via the randomForest API. - plot.gg_variable: set outcome factor levels from gg_dta_y column order rather than factor() default (alphabetical), so multi-class facet panels follow the model's class ordering regardless of locale. - Add two new tests: oob=FALSE warning, and outcome factor level order. Co-Authored-By: Claude Sonnet 4.5 <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.
Summary
dev/plans/2026-05-21-rf-finishing-work-design.md— covers both PRs, problem statements, exact before/after code, test cases, files-touched table, non-goalsdev/plans/2026-05-21-rf-87-gg-variable-classification-plan.md— TDD plan forgg_variable.randomForestclassification fix + smooth bugs + stale gg_roc()/calc_roc() produces a degenerate ROC (~0.5 AUC, 3 points) for randomForest objects #81/Meta: validate the randomForest engine (plot/ROC correctness + regression coverage) #82 closuredev/plans/2026-05-21-rf-88-multiclass-roc-plan.md— TDD plan forgg_rocper_class=TRUEfeature (closes Meta: gg_roc enhancements (multi-class + confidence intervals) #72)No code changes in this PR — docs/plans only.
🤖 Generated with Claude Code