Fold v2.7.1 follow-ups: news source consolidation + vimp legend merge

* ggrandomforests.news() now reads NEWS.md (the canonical change log R
  also exposes via utils::news()). Removes inst/NEWS, which had silently
  drifted to v2.4.0 across three releases -- users running the helper
  were seeing year-old version info. Adds a fallback to inst/NEWS for
  legacy installs and a regression test that the bundled NEWS.md
  contains the installed package version.
* plot.gg_vimp(): merge the duplicate legend. Previously fill and color
  were both mapped to `positive` but only the fill legend got a custom
  title via labs(), leaving a second redundant legend titled "positive"
  (the column name). Both aesthetics now share the "VIMP > 0" title by
  default so ggplot collapses them into a single legend. Regression
  test asserts p$labels$fill == p$labels$colour == "VIMP > 0".

No DESCRIPTION bump -- v2.7.1 has not been tagged or submitted to CRAN
yet; these fold into the same release.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: ehrlinger

NEWS.md

@@ -26,6 +26,16 @@ ggRandomForests v2.7.1
   non-empty layers for every supported forest family. Catches the
   empty-figure class of bug (transform/plot column-name mismatch) without
   requiring visual inspection.
+* `ggrandomforests.news()` now reads `NEWS.md` (the canonical change log
+  R also surfaces via `utils::news()`). The legacy hand-maintained
+  `inst/NEWS` has been removed — it had silently drifted to v2.4.0
+  (June 2025) across three releases, so users running the helper saw
+  stale version info. One source of truth, no more drift window.
+* Fix `plot.gg_vimp()` legend duplication: the bar geom mapped both
+  `fill` and `color` to the `positive` column, but only the fill legend
+  was titled "VIMP > 0", leaving a redundant second legend titled
+  "positive". Both aesthetics now share the "VIMP > 0" title so ggplot
+  merges them into a single legend by default.
 
 ggRandomForests v2.7.0
 =====================

R/ggrandomforests.news.R

@@ -1,15 +1,29 @@
 #' Display the ggRandomForests NEWS file
 #'
-#' Opens the package NEWS file in the system pager so users can read the
-#' version history and change log without leaving their R session.
+#' Opens the package change log in the system pager so users can read the
+#' version history without leaving their R session. The function reads
+#' \code{NEWS.md} from the installed package root (the canonical change log
+#' that R also surfaces via \code{utils::news()}). \code{inst/NEWS} was
+#' removed in v2.7.1 to eliminate the duplicate-source-of-truth maintenance
+#' hole that left the legacy file frozen at v2.4.0; if any installation
+#' still ships an \code{inst/NEWS}, this function falls back to it.
 #'
 #' @param ... Currently unused; reserved for future arguments.
 #'
 #' @return Called for its side-effect of opening the NEWS file in the system
-#'   pager (\code{file.show}).  Returns \code{invisible(NULL)}.
+#'   pager (\code{file.show}). Returns \code{invisible(NULL)}.
 #'
 #' @keywords internal
 ggrandomforests.news <- function(...) {
-  newsfile <- file.path(system.file(package="ggRandomForests"), "NEWS")
+  newsfile <- system.file("NEWS.md", package = "ggRandomForests")
+  if (!nzchar(newsfile)) {
+    # Fallback for legacy installs that still bundle inst/NEWS.
+    newsfile <- system.file("NEWS", package = "ggRandomForests")
+  }
+  if (!nzchar(newsfile)) {
+    warning("Could not locate a NEWS file for ggRandomForests.",
+            call. = FALSE)
+    return(invisible(NULL))
+  }
   file.show(newsfile)
-}
+}

R/plot.gg_vimp.R

@@ -89,7 +89,11 @@ plot.gg_vimp <- function(x, relative, lbls, ...) {
 
   # When there are both positive and negative VIMP values, colour the bars
   # differently so the user can immediately see which variables are below zero
-  # (i.e. hurt predictive accuracy on average).
+  # (i.e. hurt predictive accuracy on average). Both `fill` and `color` are
+  # mapped to the same column; we give them the same legend title ("VIMP > 0")
+  # so ggplot collapses them into a single legend rather than rendering one
+  # legend titled "VIMP > 0" (fill) and a second titled "positive" (color).
+  legend_title <- "VIMP > 0"
   if (length(unique(gg_dta$positive)) > 1) {
     gg_plt <- gg_plt +
       ggplot2::geom_bar(
@@ -112,7 +116,11 @@ plot.gg_vimp <- function(x, relative, lbls, ...) {
         width = .5,
       )
   }
-  gg_plt <- gg_plt + labs(x = "", y = msr)
+  # Set both legends' titles to the same string so ggplot merges them.
+  # Users can override with their own labs() call after the fact.
+  gg_plt <- gg_plt +
+    ggplot2::labs(x = "", y = msr,
+                  fill = legend_title, color = legend_title)
 
   if (!missing(lbls)) {
     # Map internal variable names to human-readable labels.  lbls should be a

inst/NEWS

@@ -1,11 +1,35 @@
-# Tests for ggrandomforests.news
+# Tests for ggrandomforests.news()
+#
+# v2.7.1 consolidated the helper to read NEWS.md (the canonical change log
+# R also exposes via utils::news()) instead of the legacy inst/NEWS plain
+# text file. These tests guard against silent drift back to a separate
+# hand-maintained source.
 
-test_that("ggrandomforests.news NEWS file exists in package", {
-  newsfile <- file.path(system.file(package = "ggRandomForests"), "NEWS")
-  expect_true(file.exists(newsfile))
+test_that("NEWS.md is bundled with the installed package", {
+  nf <- system.file("NEWS.md", package = "ggRandomForests")
+  expect_true(nzchar(nf))
+  expect_true(file.exists(nf))
+})
+
+test_that("Bundled NEWS.md reflects the installed package version", {
+  # Reading NEWS.md and finding the DESCRIPTION version in it ensures the
+  # change-log entry was written before the version bump landed — i.e. that
+  # users running ggrandomforests.news() see the correct release.
+  ver <- as.character(utils::packageVersion("ggRandomForests"))
+  nf  <- system.file("NEWS.md", package = "ggRandomForests")
+  txt <- readLines(nf, warn = FALSE)
+  expect_true(any(grepl(ver, txt, fixed = TRUE)),
+              info = sprintf("Installed version %s not found in %s", ver, nf))
 })
 
 test_that("ggrandomforests.news is callable without error", {
-  # file.show opens the file but should not throw an error
   expect_error(ggrandomforests.news(), NA)
 })
+
+test_that("inst/NEWS is no longer the source of truth", {
+  # Defensive: if a future change re-introduces inst/NEWS, this test won't
+  # fail (the helper still falls back to it) -- but we explicitly assert
+  # NEWS.md takes priority when both exist.
+  nf_md <- system.file("NEWS.md", package = "ggRandomForests")
+  expect_true(nzchar(nf_md))
+})

man/ggrandomforests.news.Rd

@@ -287,6 +287,31 @@ test_that("plot.gg_vimp renders one bar per variable", {
   expect_equal(nrow(ld), length(rf$xvar.names))
 })
 
+test_that("plot.gg_vimp produces a single merged 'VIMP > 0' legend", {
+  # Regression: previously the bar geom mapped both `fill` and `color` to
+  # `positive` but only the fill legend got a custom title via labs(). ggplot
+  # rendered two legends -- one labelled "VIMP > 0" (fill) and one labelled
+  # "positive" (color, the column name). Setting matching titles on both
+  # aesthetics merges them into one.
+  data(pbc, package = "randomForestSRC")
+  pbc_min <- pbc[!is.na(pbc$treatment), ]
+  set.seed(42)
+  rf <- randomForestSRC::rfsrc(
+    Surv(days, status) ~ ., data = pbc_min,
+    ntree = 100, importance = TRUE, na.action = "na.impute"
+  )
+  gg <- gg_vimp(rf)
+  # Sanity: ensure both signs of VIMP are present so the fill aesthetic is
+  # actually in play.
+  expect_true(length(unique(gg$positive)) > 1)
+
+  p <- plot(gg)
+  expect_equal(p$labels$fill,   "VIMP > 0")
+  expect_equal(p$labels$colour, "VIMP > 0")
+  # The two aesthetic legend titles must match for ggplot to merge them.
+  expect_identical(p$labels$fill, p$labels$colour)
+})
+
 # ----------------------------------------------------------------------------
 # gg_variable
 # ----------------------------------------------------------------------------