Improves DimHeatmap documentation and default behavior when fast=FALSE (Issue #9810) by dkcoxie · Pull Request #9813 · satijalab/seurat

dkcoxie · 2025-04-10T21:05:46Z

Clarifies documentation to specify that figure legends are currently not displayed when fast=TRUE but added titles with PC numbers to ggplot outputs so that both PC number and figure legends can be displayed on the same plot, plus option to move position of shared patchwork legend when combine=TRUE to resolve Issue #9810

Clarifies documentation to specify that figure legends are currently not displayed when `fast=TRUE` but added titles with PC numbers to ggplot outputs so that both PC number and figure legends can be displayed on the same plot, plus option to move position of shared patchwork legend when `combine=TRUE` to resolve Issue [satijalab#9810](satijalab#9810)

dkcoxie · 2025-04-10T21:12:30Z

In preparing teaching materials geared toward beginners, we found that the default behavior of DimHeatmap() either displays no figure legend but includes the PC number as panel titles (default fast=TRUE) OR allows the figure legend to be shown for the ggplot2 output doesn't display the PC number for each panel (when fast=FALSE).

The Proposed changes to theDimHeatmap() function clarify the documentation regarding when a figure legend will be displayed, add PC labels as individual figure titles for the ggplot2 outputs, and allow for the position of the figure legend to be moved when the ggplots are combined into a single patchwork output.

dkcoxie · 2025-07-10T16:27:42Z

The related issue was closed so I wanted to check if this improvement to make the output plots more consistent for DimHeatmap() when fast=TRUE and when fast=FALSE in still being considered for the next CRAN release. Thank you!

anashen · 2025-09-11T16:44:17Z

Thanks @dkcoxie for your PR & for the follow-up :) Yes, we have added this to the list of PRs to review.

anashen

Thank you @dkcoxie for opening this PR! These changes will definitely be helpful to add. I made a couple of suggestions & would appreciate it if you could take a look.

R/visualization.R

Updated manual message to suggested wording Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Updating documentation to reflect change to parameter name for better clarity. Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Changing legend parameter name for better clarity. Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Changing legend parameter name within function to improve clarity. Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Copilot

Pull request overview

This PR enhances the DimHeatmap function to better document its behavior and improve visualization output when using ggplot2 mode (fast=FALSE). The changes address Issue #9810 by adding PC number titles to individual heatmap plots and providing control over legend positioning in combined plots.

Changes:

Updated documentation to clarify that fast=TRUE excludes figure legends from output
Added PC number titles (e.g., "PC_1") to each heatmap panel when fast=FALSE with centered, bold formatting
Introduced leg.pos parameter to control legend position in combined patchwork outputs, defaulting to "right"

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

R/visualization.R

Added suggested import to resolve error if patchwork is not already loaded by user

dkcoxie · 2026-02-19T18:11:35Z

@anashen - made the requested changes to my branch for improving the heatmap plotting behavior and documentation. Ignoring the Copilot suggestions since the majority seem to be redundant with the suggestion but happy to make additional changes if needed or change merge destination from main to a development branch.

Copilot

Pull request overview

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

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

Copilot · 2026-02-19T18:18:27Z

R/visualization.R

-#' ggplot object. If \code{FALSE}, return a list of ggplot objects
+#' @param combine Combine plots into a single \code{\link[patchwork]{patchwork}ed} ggplot object with 
+#' single shared figure legend when \code{fast=FALSE}. If \code{FALSE}, return a list of ggplot objects
+#' @param legend.position When \code{combine=TRUE}, allows legend position to be adjusted for \code{\link[patchwork]{patchwork}ed} output; defaults as \code{"right"}


Incorrect grammar in documentation. The phrase "defaults as" should be "defaults to" for grammatically correct documentation.

Suggested change

#' @param legend.position When \code{combine=TRUE}, allows legend position to be adjusted for \code{\link[patchwork]{patchwork}ed} output; defaults as \code{"right"}

#' @param legend.position When \code{combine=TRUE}, allows legend position to be adjusted for \code{\link[patchwork]{patchwork}ed} output; defaults to \code{"right"}

Copilot · 2026-02-19T18:18:28Z

R/visualization.R

 #' returns a list of ggplot objects
 #'
-#' @importFrom patchwork wrap_plots
+#' @importFrom patchwork wrap_plots plot_layout


Missing required ggplot2 imports. The code now uses ggtitle, theme, and element_text functions (lines 177-178), but these are not imported in the function documentation. Add ggtitle theme element_text to the importFrom directive to ensure these functions are available.

Copilot · 2026-02-19T18:18:28Z

R/visualization.R

+    plots <- wrap_plots(plots, ncol = ncol, guides = "collect") + 
+      plot_layout(guides = "collect") &


Incorrect patchwork syntax with redundant plot_layout call. The code uses both + and & operators with redundant guides = "collect" specification. When using wrap_plots with guides = "collect", you don't need to add a separate plot_layout(guides = "collect"). The & operator should be used directly after wrap_plots to apply themes to all subplots. Simplify to: plots <- wrap_plots(plots, ncol = ncol, guides = "collect") & theme(legend.position = legend.position)

Suggested change

plots <- wrap_plots(plots, ncol = ncol, guides = "collect") +

plot_layout(guides = "collect") &

plots <- wrap_plots(plots, ncol = ncol, guides = "collect") &

Copilot · 2026-02-19T18:18:28Z

R/visualization.R

+    assays = NULL,
+    combine = TRUE,
+    legend.position = "right"
 ) {


Missing input validation for legend.position parameter. The new legend.position parameter accepts any value without validation. Consider adding validation to ensure only valid ggplot2 legend positions are accepted (e.g., "none", "left", "right", "bottom", "top", or a numeric vector of length 2). This would provide better error messages to users who might provide invalid values.

Suggested change

) {

) {

# Validate legend.position to ensure it matches accepted ggplot2 values

if (is.character(legend.position)) {

valid_positions <- c("none", "left", "right", "bottom", "top")

if (length(legend.position) != 1L || !legend.position %in% valid_positions) {

stop(

"legend.position must be one of ",

paste(shQuote(valid_positions), collapse = ", "),

" when supplied as a character string.",

call. = FALSE

)

}

} else if (is.numeric(legend.position)) {

if (length(legend.position) != 2L) {

stop(

"legend.position must be a numeric vector of length 2 when supplied as numeric.",

call. = FALSE

)

}

} else {

stop(

"legend.position must be either a character string (\"none\", \"left\", \"right\", \"bottom\", \"top\") ",

"or a numeric vector of length 2.",

call. = FALSE

)

}

Copilot · 2026-02-19T18:18:28Z

R/visualization.R

  if (combine) {
-    plots <- wrap_plots(plots, ncol = ncol, guides = "collect")
+    plots <- wrap_plots(plots, ncol = ncol, guides = "collect") + 
+      plot_layout(guides = "collect") &
+      theme(legend.position = legend.position)


The legend.position parameter only affects the output when combine=TRUE, but this is not checked in the code. When combine=FALSE, the parameter is silently ignored. Consider either: (1) applying the legend position to individual plots in the list when combine=FALSE, or (2) adding a warning when legend.position is specified but combine=FALSE, or (3) clarifying in the documentation that this parameter has no effect when combine=FALSE.

anashen self-requested a review September 11, 2025 16:44

anashen changed the base branch from develop to main September 23, 2025 18:16

anashen linked an issue Sep 23, 2025 that may be closed by this pull request

Feature Request: Improve DimHeatmap documentation and add PC number title for ggplot2 output #9810

Closed

anashen requested changes Sep 25, 2025

View reviewed changes

R/visualization.R Outdated Show resolved Hide resolved

R/visualization.R Outdated Show resolved Hide resolved

R/visualization.R Outdated Show resolved Hide resolved

R/visualization.R Outdated Show resolved Hide resolved

R/visualization.R Show resolved Hide resolved

Merge branch 'main' into dkcoxie-patch-1

75aef9e

Copilot AI review requested due to automatic review settings February 19, 2026 17:58

Copilot started reviewing on behalf of dkcoxie February 19, 2026 17:58 View session

dkcoxie and others added 4 commits February 19, 2026 13:00

Update R/visualization.R

66a463e

Updated manual message to suggested wording Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Update R/visualization.R

7867c80

Updating documentation to reflect change to parameter name for better clarity. Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Update R/visualization.R

3bb8c84

Changing legend parameter name for better clarity. Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Update R/visualization.R

f154c7a

Changing legend parameter name within function to improve clarity. Co-authored-by: Anagha Shenoy <90294056+anashen@users.noreply.github.com>

Copilot AI reviewed Feb 19, 2026

View reviewed changes

R/visualization.R Show resolved Hide resolved

R/visualization.R Show resolved Hide resolved

R/visualization.R Show resolved Hide resolved

R/visualization.R Show resolved Hide resolved

Add plot_layout import from patchwork

21947e6

Added suggested import to resolve error if patchwork is not already loaded by user

dkcoxie requested a review from Copilot February 19, 2026 18:12

Copilot started reviewing on behalf of dkcoxie February 19, 2026 18:13 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

dkcoxie requested a review from anashen February 19, 2026 19:14

	#' @param legend.position When \code{combine=TRUE}, allows legend position to be adjusted for \code{\link[patchwork]{patchwork}ed} output; defaults as \code{"right"}
	#' @param legend.position When \code{combine=TRUE}, allows legend position to be adjusted for \code{\link[patchwork]{patchwork}ed} output; defaults to \code{"right"}

		plots <- wrap_plots(plots, ncol = ncol, guides = "collect") +
		plot_layout(guides = "collect") &

-) {
+) {
+  # Validate legend.position to ensure it matches accepted ggplot2 values
+  if (is.character(legend.position)) {
+    valid_positions <- c("none", "left", "right", "bottom", "top")
+    if (length(legend.position) != 1L || !legend.position %in% valid_positions) {
+      stop(
+        "legend.position must be one of ",
+        paste(shQuote(valid_positions), collapse = ", "),
+        " when supplied as a character string.",
+        call. = FALSE
+      )
+    }
+  } else if (is.numeric(legend.position)) {
+    if (length(legend.position) != 2L) {
+      stop(
+        "legend.position must be a numeric vector of length 2 when supplied as numeric.",
+        call. = FALSE
+      )
+    }
+  } else {
+    stop(
+      "legend.position must be either a character string (\"none\", \"left\", \"right\", \"bottom\", \"top\") ",
+      "or a numeric vector of length 2.",
+      call. = FALSE
+    )
+  }

Conversation

dkcoxie commented Apr 10, 2025

Uh oh!

dkcoxie commented Apr 10, 2025

Uh oh!

dkcoxie commented Jul 10, 2025

Uh oh!

anashen commented Sep 11, 2025

Uh oh!

anashen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dkcoxie commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants