Fix empty-figure bugs in gg_partial_rfsrc (release v2.7.1)#70
Merged
Conversation
Survival forests in gg_partial_rfsrc() were calling partial.rfsrc() without partial.type, triggering a zero-length comparison inside randomForestSRC's C-level prediction code that aborted the call -- which is why the survival vignette's partial-dependence chunks rendered no figures. * Pass partial.type = "surv" by default for survival forests; expose it as a new gg_partial_rfsrc() argument accepting "surv" / "chf" / "mort". * Reshape multi-partial.time results to long form: get.partial.plot.data() returns yhat as a [length(partial.values) x length(partial.time)] matrix, but the previous code assumed a vector and crashed on the time-column assignment. * Improve plot.gg_partial_rfsrc() survival layout: predictor on x-axis, one curve per (rounded) time point coloured by Time, faceted by name. The previous default put time on the x-axis with one near-identical line per predictor value. * New tests/testthat/test_plot_layer_data.R uses ggplot2::layer_data() to verify each plot.gg_*() method renders non-empty layers across all forest families (regression / classification / survival, with/without conf.int, by, multi-time partial-dep). Catches empty-figure regressions without visual inspection. Bump DESCRIPTION to 2.7.1 (semver patch -- bug-fix only) and refresh cran-comments.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
+ Coverage 83.59% 84.30% +0.71%
==========================================
Files 25 25
Lines 2005 2032 +27
==========================================
+ Hits 1676 1713 +37
+ Misses 329 319 -10
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Bug-fix patch release addressing an “empty figure” failure mode in survival partial-dependence plots by ensuring randomForestSRC::partial.rfsrc() is called with the required partial.type, and by correctly reshaping multi-time partial dependence output for plotting.
Changes:
- Add
partial.typeargument togg_partial_rfsrc()(defaulting to"surv"for survival forests) and pass it through torandomForestSRC::partial.rfsrc(). - Reshape multi-
partial.timesurvival partial-dependenceyhatmatrix output into long form. - Add a regression test suite using
ggplot2::layer_data()to ensureplot.gg_*()methods produce non-empty rendered layers across forest families.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
R/gg_partial_rfsrc.R |
Adds partial.type handling for survival forests and reshapes multi-time partial output into long form. |
R/plot.gg_partial.R |
Updates survival partial-dependence plot layout to use predictor on x-axis and time as the curve grouping/color. |
tests/testthat/test_plot_layer_data.R |
Adds regression tests that validate plots render non-empty layer data (prevents “empty figure” regressions). |
man/gg_partial_rfsrc.Rd |
Documents new partial.type argument. |
DESCRIPTION |
Bumps package version/date for v2.7.1. |
NEWS.md |
Adds v2.7.1 release notes describing the fixes and new tests. |
cran-comments.md |
Updates CRAN submission notes for v2.7.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+126
to
+128
| ggplot2::geom_line() + | ||
| ggplot2::facet_wrap(~name, scales = "free") + | ||
| ggplot2::labs(x = "Time", y = "Partial Effect", color = "Predictor value") | ||
| ggplot2::facet_wrap(~name, scales = "free_x") + | ||
| ggplot2::labs(x = NULL, y = "Predicted Survival", color = "Time") |
Comment on lines
+111
to
+113
| if (!is.null(cont$time)) { | ||
| ## Survival forest: predictor value is the grouping variable; x-axis is time | ||
| ## Survival forest: predictor value on x-axis, one curve per time point | ||
| ## (rounded for a tidy legend). Time is typically a small set (1-3 horizons) |
Comment on lines
115
to
124
| cont$.time_lbl <- factor(round(cont$time, 2), | ||
| levels = sort(unique(round(cont$time, 2)))) | ||
| gg_cont <- ggplot2::ggplot( | ||
| cont, | ||
| ggplot2::aes( | ||
| x = .data$time, | ||
| x = .data$x, | ||
| y = .data$yhat, | ||
| color = factor(.data$x), | ||
| group = factor(.data$x) | ||
| color = .data$.time_lbl, | ||
| group = .data$.time_lbl | ||
| ) |
* plot.gg_partial_rfsrc(): y-axis label now adapts to partial.type. The attribute is stamped on the gg_partial_rfsrc object and read back via a new partial_surv_y_label() helper that maps "surv" -> "Predicted Survival", "chf" -> "Predicted CHF", "mort" -> "Predicted Mortality" (with a "Predicted Survival" fallback for legacy objects without the attribute). * plot.gg_partial_rfsrc(): group/colour by full-precision time (not the rounded label), so distinct time horizons that round to the same value no longer collapse into a single curve. Rounding is applied only to legend labels via scale_color_discrete(labels = ...). * plot.gg_partial_rfsrc() roxygen: revised section header to match the new layout (one curve per evaluation time over the predictor's value; y-axis adapts to partial.type). * tests: lock in the y-label dispatch for "surv" / "chf" / "mort" and the full-precision time grouping (synthetic two-time object with values that round to the same 2-dp display). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Addressed all three Copilot review items in 3216c92:
Locked in by two new tests in
|
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
Bug-fix release. The survival vignette's partial-dependence figures were
rendering empty because
gg_partial_rfsrc()calledrandomForestSRC::partial.rfsrc()withoutpartial.typefor survivalforests, triggering a zero-length comparison
(
if (partial.type == \"rel.freq\") ...) inside the C-level predictionroutine and aborting the call. The chunk had `error=TRUE`, so the
failure surfaced as missing figures rather than an error.
A second latent bug surfaced once the first was fixed: with multiple
`partial.time` values, `get.partial.plot.data()` returns yhat as an
`[length(partial.values) × length(partial.time)]` matrix, but the code
assumed a vector and crashed on the `time` column assignment.
Fixes
it as a new `gg_partial_rfsrc()` argument (`"surv"` / `"chf"` /
`"mort"`).
`(x, time)` pair is one row.
the x-axis with one curve per (rounded) time point coloured by
`Time`, faceted by variable name. Previous layout produced a
saturated legend with dozens of nearly-identical lines.
Empty-figure regression suite
New `tests/testthat/test_plot_layer_data.R` (57 assertions) uses
`ggplot2::layer_data()` to verify each `plot.gg_*()` method renders
non-empty layers across every supported forest family — regression,
classification, survival, with/without `conf.int` / `by` / multi-time
partial-dependence. Catches transform/plot column-name mismatches
structurally without visual inspection.
Release
`Date: 2026-04-27`.
Test plan
re-building enabled (Status: OK; 0 errors / 0 warnings / 0 notes).
`cell-output cell-output-error` divs in the resulting HTML.
expected per-variable curves at the requested time horizons.
🤖 Generated with Claude Code