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

add seed argument to position_jitterdodge #2445

Merged
merged 4 commits into from Feb 12, 2018

Conversation

Projects
None yet
2 participants
@slowkow
Contributor

slowkow commented Feb 9, 2018

In pull #1996 position_jitter got a new argument called seed.

position_jitterdodge might also benefit from having a seed.

Below, I show an example of position_jitterdodge before and after my changes.

Please notice two things:

  1. After my changes, the text labels are correctly centered on the points.

  2. The x-axis gains 2 tick marks at 5 and 7.

    • This was unexpected, and I don't know why it happened or if it matters much. It's easy to fix by doing dat$cyl <- factor(dat$cyl).

Here's the code:

library(ggplot2)
dat <- mtcars
dat$name <- rownames(mtcars)
dat$name[dat$cyl != 6] <- ""

set.seed(1)
pos <- position_jitterdodge(seed = 1)
ggplot(dat, aes(cyl, mpg, color = name != "", label = name)) +
  geom_point(
    size = 0.5,
    position = pos
  ) +
  geom_text(
    size = 2,
    position = pos
  )
# ggplot2      * 2.2.1.9000 2018-02-09 Github (tidyverse/ggplot2@2b5b88d)
# ggsave("ggplot2-2b5b88d-jitterdodge.png", width = 6, height = 4)
ggsave("ggplot2-slowkow-jitterdodge.png", width = 6, height = 4)

Before

This figure was made with ggplot2 2b5b88d

ggplot2-2b5b88d-jitterdodge

After

This figure was made after making the changes in this pull request.

ggplot2-slowkow-jitterdodge

add seed argument to position_jitterdodge
In pull #1996 position_jitter got a new argument called seed.

position_jitterdodge might also benefit from having a seed.
@@ -10,19 +10,31 @@
#' @param jitter.height degree of jitter in y direction. Defaults to 0.
#' @param dodge.width the amount to dodge in the x direction. Defaults to 0.75,
#' the default `position_dodge()` width.
#' @param seed A random seed to make the jitter reproducible.

This comment has been minimized.

@hadley

hadley Feb 10, 2018

Member

Can you inherit these docs from geom_jitter()?

@hadley

This comment has been minimized.

Member

hadley commented Feb 10, 2018

Can you please also modify the NEWS bullet added for the position_jitter() change? (IIRC that it's only in the dev version)

@slowkow slowkow referenced this pull request Feb 11, 2018

Closed

labels for geom_jitter #52

@slowkow

This comment has been minimized.

Contributor

slowkow commented Feb 11, 2018

position_jitterdodge now inherits the seed documentation from position_jitter.

I'm not sure if I modified NEWS.md the way you wanted. Let me know if I should change it.

I also realized that my example above is not clear. I hope the example below is much clearer.

Before

devtools::install_github("tidyverse/ggplot2@2b5b88d")
library(ggplot2)

set.seed(2)
pos <- position_jitterdodge(jitter.width = 0.5, dodge.width = 1)
ggplot(mtcars, aes(cyl, mpg, color = factor(am), group = am)) +
  geom_point(position = pos) +
  geom_point(size = 4, shape = 1, position = pos) +
  coord_cartesian(xlim = c(5, 7), ylim = c(16, 23)) +
  labs(title = "position_jitterdodge is not ok")
ggsave("ggplot2-2b5b88d-jitterdodge.png", width = 4, height = 3)

ggplot2-2b5b88d-jitterdodge

After

library(ggplot2)

pos <- position_jitterdodge(jitter.width = 0.5, dodge.width = 1, seed = 2)
ggplot(mtcars, aes(cyl, mpg, color = factor(am), group = am)) +
  geom_point(position = pos) +
  geom_point(size = 4, shape = 1, position = pos) +
  coord_cartesian(xlim = c(5, 7), ylim = c(16, 23)) +
  labs(title = "position_jitterdodge is ok")
ggsave("ggplot2-pull2445-jitterdodge.png", width = 4, height = 3)

ggplot2-pull2445-jitterdodge

@hadley

This comment has been minimized.

Member

hadley commented Feb 11, 2018

Can you please combine the two news bullets into one?

@slowkow

This comment has been minimized.

Contributor

slowkow commented Feb 11, 2018

News bullets are combined into one.

@hadley hadley merged commit 39e4a3b into tidyverse:master Feb 12, 2018

2 of 4 checks passed

codecov/patch 0% of diff hit (target 75.02%)
Details
codecov/project 74.98% (-0.05%) compared to 2b5b88d
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hadley

This comment has been minimized.

Member

hadley commented Feb 12, 2018

Perfect, thanks!

@lock

This comment has been minimized.

lock bot commented Aug 11, 2018

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/

@lock lock bot locked and limited conversation to collaborators Aug 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.