Skip to content

Use ggplot2 for plotEISA(), add identity line and set aspect ratio to 1 - #43

Merged
mbstadler merged 3 commits into
fix-testsfrom
ggplot
Jul 28, 2026
Merged

Use ggplot2 for plotEISA(), add identity line and set aspect ratio to 1#43
mbstadler merged 3 commits into
fix-testsfrom
ggplot

Conversation

@csoneson

Copy link
Copy Markdown
Collaborator

No description provided.

@csoneson
csoneson requested a review from mbstadler July 20, 2026 20:02

@mbstadler mbstadler left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks all great. Two minor thoughts:

  • would it make sense to keep the ellipsis to avoid potential breaking of existing code? (see also in-line comment)
  • should we add something to the NEWS file?

Comment thread R/plotEISA.R
#' @export
plotEISA <- function(x, contrast = c("ExIn", "none"),
minLfc = NULL, maxFDR = 0.05,
genecolors = c("#E41A1C", "#497AB3", "#222222"), ...) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming some people use plotEISA(...) with additional graphical arguments in the ellipsis to be passed to plot(...), this will be a breaking change. What about keeping the ellipsis, but checking if it is non-empty and throw a warning in that case?

@csoneson

Copy link
Copy Markdown
Collaborator Author

Yes, both good points, thanks - I added back the ellipsis and updated the NEWS file

@mbstadler
mbstadler merged commit 2865396 into fix-tests Jul 28, 2026
2 checks passed
@mbstadler
mbstadler deleted the ggplot branch July 28, 2026 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants