-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix for issue #825 so that dotplot now works with qplot. #912
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
Conversation
Thanks! Can you please complete the rest of this checklist:
@wch does this seem reasonable? |
@@ -176,8 +176,13 @@ GeomDotplot <- proto(Geom, { | |||
|
|||
# Next part will set the position of each dot within each stack | |||
# If stackgroups=TRUE, split only on x (or y) and panel; if not stacking, also split by group | |||
plyvars <- c(params$binaxis, "PANEL") | |||
if (!params$stackgroups) | |||
if(is.null(params$binaxis)) { |
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.
This could be plyvars <- params$binaxis %||% "x"
Yo, I've added a row to NEWS and rewritten the patch to use %||% . No minimal example since it's no graphical feature, I guess. No new documentation either... Cheers! |
Can you make it |
I've added an example both to the qplot and the geom_dotplot documentation and changed the NEWS. Was this how you meant? |
@@ -95,6 +95,9 @@ | |||
#' | |||
#' ggplot(mtcars, aes(x = 1, y = mpg, fill = factor(cyl))) + | |||
#' geom_dotplot(binaxis = "y", stackgroups = TRUE, binwidth = 1, method = "histodot") | |||
#' | |||
#' # Use qplot instead | |||
#' qplot(mpg, data=mtcars, geom="dotplot") |
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.
Last tiny change: can you please put spaces around the equal signs?
My bad. Fixed! |
And now, can you please merge/rebase so I can cleanly merge? (It's probably just a conflict in NEWS). Thanks! |
And now I messed it up, sorry. I'll try to sort it out... |
…till works when no params are passed in (as when using qplot).
Like this? |
Fix for issue #825 so that dotplot now works with qplot.
Perfect - thanks! |
:) |
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
This is a fix for issue #825.
It seems that when a dotplot is created using
geom_dotplot
a couple of parameters (binaxis, stackdir, stackgroups, etc.) are assigned default values. When a dotplot is created usingqplot(mpg, data=mtcars, geom="dotplot")
these params are not defined. Thereparameterise
function of GeomDotplot expect some of these params to be defined thus dotplot does not work with qplot.The easy fix for this for
reparameterise
to check if these params are defined and if not use defaults. I've implemented these checks and have used the same defaults as used in thegeom_dotplot
function. Using this fixqplot(mpg, data=mtcars, geom="dotplot")
does not result in an error and gives the same output as
ggplot(mtcars, aes(x = mpg)) + geom_dotplot()
#825