FrankPortman / bayesAB

🐢 bayesAB: Fast Bayesian Methods for A/B Testing
http://frankportman.github.io/bayesAB/
Other
307 stars 41 forks source link

Forthcoming ggplot2 release and bayesAB #33

Closed paleolimbot closed 5 years ago

paleolimbot commented 5 years ago

We're in the process of preparing a ggplot2 release. As part of the release process, we run the R CMD check on packages that use ggplot2 to make sure we don't accidentally break code for downstream packages.

In running the R CMD check on bayesAB, we identified the following issue:

These failures are because bayesAB is using geom_ribbon() without layer data. The behaviour of layers when "setting" an aesthetic (outside aesI()) with length > 1 is not defined and may change in the future (in the case of geom_ribbon(), we now require that at least one of ymin or ymax is mapped to prevent bugs from occurring).

To fix this error, we suggest using a geom_*() function with its own data. Note that using .data$col_name within aes() is the preferred way to avoid CMD check issues about undefined variables when mapping columns (make sure you include #' importFrom rlang .data in at least one roxygen documentation block to avoid an error about the name .data being undefined).

ggplot2::ggplot() +
  ggplot2::geom_ribbon(
    ggplot2::aes(x = .data$x, ymax = .data$ymax),
    data = data.frame(x = 1:5, ymax = c(0, 1, 3, 1, 0)),
    ymin = 0
  )

Created on 2019-05-09 by the reprex package (v0.2.1)

We hope to release the new version of ggplot2 in the next two weeks, at which point you will get a note from CRAN that your package checks are failing. Let me know if I can help!

FrankPortman commented 5 years ago

Fixed by https://github.com/FrankPortman/bayesAB/pull/34.

https://github.com/tidyverse/ggplot2/pull/3303#issuecomment-489715276 in the spirit of this comment I'd like to be referred to in the future as "Torturer of APIs" 😄