Skip to content
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

stat_contour's breaks parameter is undocumented #2472

Closed
alistaire47 opened this issue Mar 5, 2018 · 4 comments · Fixed by #3423
Closed

stat_contour's breaks parameter is undocumented #2472

alistaire47 opened this issue Mar 5, 2018 · 4 comments · Fixed by #3423
Labels
documentation good first issue ❤️ help wanted ❤️ tidy-dev-day 🤓

Comments

@alistaire47
Copy link
Contributor

@alistaire47 alistaire47 commented Mar 5, 2018

When trying to use geom_contour to draw lines at specific levels à la contour(..., levels = ...), I discovered this SO answer which documents that geom_contour can be passed a breaks parameter to specify particular z values where contour lines should be drawn.

Weirdly, this behavior is not documented in ?geom_contour, ?geom_density_2d, ?layer, or ?ggplot2:::StatContour, and taking a cursory look at the source, I'm not sure where the breaks parameter is picked up. Given the behavior is important enough to merit a parameter in contour, seems like it should be in the docs.

I can make a PR if you like, but the problem is that I'm not exactly sure how this behavior should be documented, as I'm not sure what breaks is a parameter of. An example wouldn't be too hard to add, though.

Reprex:

library(ggplot2)

v <- ggplot(faithfuld, aes(waiting, eruptions, z = density)) 

v + geom_contour()

v + geom_contour(breaks = c(0.002, 0.02))

@alistaire47 alistaire47 changed the title stat_contour's breaks parameter is undocumented stat_contour's breaks parameter is undocumented Mar 5, 2018
@hadley hadley added good first issue ❤️ help wanted ❤️ labels Apr 27, 2018
@clauswilke
Copy link
Member

@clauswilke clauswilke commented May 9, 2018

The bins, binwidth, breaks, and complete parameters are parameters of stat_contour():

ggplot2/R/stat-contour.r

Lines 37 to 38 in d3bd92e

compute_group = function(data, scales, bins = NULL, binwidth = NULL,
breaks = NULL, complete = FALSE, na.rm = FALSE) {

(complete has no effect, though, as far as I can tell.)

They should be documented just like the comparable parameters in stat_bin(), and they should probably also be added to the stat_contour() constructor (again, just like in stat_bin()):

ggplot2/R/stat-bin.r

Lines 42 to 49 in d3bd92e

stat_bin <- function(mapping = NULL, data = NULL,
geom = "bar", position = "stack",
...,
binwidth = NULL,
bins = NULL,
center = NULL,
boundary = NULL,
breaks = NULL,

@JHonaker
Copy link

@JHonaker JHonaker commented Oct 3, 2018

I'll take a stab at fixing this.

@paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Jul 7, 2019

This TDD issue is a relatively easy fix! The open PR on this (#2926) has been abandoned and needs a few fixes, notably that

      bins = NULL,
      binwidth = NULL,
      breaks = NULL,

(in geom-contour.r) needs to be

      bins = bins,
      binwidth = binwidth,
      breaks = breaks,

(or else the user-specified bins value won't be passed on to the stat/geom), and the documentation needs to be built with devtools::document(). A fresh PR should fix this nicely!

paleolimbot pushed a commit that referenced this issue Jul 17, 2019
…ixes #2472)

* Added explicit bins, binwidth + breaks arguments and documented them in stat/geom contour
@lock
Copy link

@lock lock bot commented Jan 13, 2020

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 Jan 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation good first issue ❤️ help wanted ❤️ tidy-dev-day 🤓
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants