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

GeomTile should have a non_missing_aes with xmin, xmax, ymin, ymax #4495

Closed
sigmapi opened this issue May 29, 2021 · 0 comments · Fixed by #4683
Closed

GeomTile should have a non_missing_aes with xmin, xmax, ymin, ymax #4495

sigmapi opened this issue May 29, 2021 · 0 comments · Fixed by #4683
Labels
bug an unexpected problem or unintended behavior layers 📈

Comments

@sigmapi
Copy link

sigmapi commented May 29, 2021

Similarly to GeomBar and GeomCol that use GeomRect to draw, I believe GeomTile too should declare a non_missing_aes member for xmin, xmax, ymin, ymax variables.

For example in GeomBar I see this:

  # These aes columns are created by setup_data(). They need to be listed here so
  # that GeomRect$handle_na() properly removes any bars that fall outside the defined
  # limits, not just those for which x and y are outside the limits
  non_missing_aes = c("xmin", "xmax", "ymin", "ymax"),

I stumbled to this, using geom_bin2d (which uses GeomTile) where it may produce data with NA in any of xmin, xmax, ymin, ymax. Here is a reprex showing that result grob contains NAs.

library(ggplot2)
p <- ggplot(diamonds, aes(x, y)) + xlim(4, 10) + ylim(4, 10) + geom_bin2d()
g <- layer_grob(p, 1)[[1]]
#> Warning: Removed 478 rows containing non-finite values (stat_bin2d).
any(is.na(g$x)|is.na(g$y))
#> [1] TRUE

Created on 2021-05-18 by the reprex package (v2.0.0)

Adding the non_missing_aes fixes this.

@thomasp85 thomasp85 added bug an unexpected problem or unintended behavior layers 📈 labels Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug an unexpected problem or unintended behavior layers 📈
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants