-
Notifications
You must be signed in to change notification settings - Fork 977
Improves DimHeatmap documentation and default behavior when fast=FALSE (Issue #9810) #9813
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
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)
|
In preparing teaching materials geared toward beginners, we found that the default behavior of The Proposed changes to the |
|
The related issue was closed so I wanted to check if this improvement to make the output plots more consistent for |
|
Thanks @dkcoxie for your PR & for the follow-up :) Yes, we have added this to the list of PRs to review. |
anashen
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| #' @param combine Combine plots into a single \code{\link[patchwork]{patchwork}ed} with single shared figure legend when \code{fast=FALSE} | ||
| #' ggplot object. If \code{FALSE}, return a list of ggplot objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #' @param combine Combine plots into a single \code{\link[patchwork]{patchwork}ed} with single shared figure legend when \code{fast=FALSE} | |
| #' 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 combine Combine plots into a single \code{\link[patchwork]{patchwork}ed} | ||
| #' @param combine Combine plots into a single \code{\link[patchwork]{patchwork}ed} with single shared figure legend when \code{fast=FALSE} | ||
| #' ggplot object. If \code{FALSE}, return a list of ggplot objects | ||
| #' @param leg.pos When \code{combine=TRUE}, allows legend position to be adjusted for \code{\link[patchwork]{patchwork}ed} output; defaults as \code{"right"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| #' @param leg.pos 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 as \code{"right"} |
Renaming this parameter to legend.position would make its purpose a bit more clear to users.
| slot = 'scale.data', | ||
| assays = NULL, | ||
| combine = TRUE, | ||
| leg.pos = "right" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| leg.pos = "right" | |
| legend.position = "right" |
| plots <- wrap_plots(plots, ncol = ncol, guides = "collect") | ||
| plots <- wrap_plots(plots, ncol = ncol, guides = "collect") + | ||
| plot_layout(guides = "collect") & | ||
| theme(legend.position = leg.pos) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| theme(legend.position = leg.pos) | |
| theme(legend.position = legend.position) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function documentation will need to be updated to also import plot_layout (line 47):
#' @importFrom patchwork wrap_plots plot_layoutOtherwise, users will receive an error running DimHeatmap when the patchwork package isn't loaded (Error in plot_layout(guides = "collect") : could not find function "plot_layout").
Clarifies documentation to specify that figure legends are currently not displayed when
fast=TRUEbut 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 whencombine=TRUEto resolve Issue #9810