Skip to content

Make geom_col() ignore data$width #3194

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions R/geom-bar.r
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ GeomBar <- ggproto("GeomBar", GeomRect,
non_missing_aes = c("xmin", "xmax", "ymin", "ymax"),

setup_data = function(data, params) {
# Contrary to the case of geom_col(), the `width` in `data` is very likely
# a calculated column by Stat. So, geom_bar() should use data$width if
# available. Note that StatIdentity is not the case, but, as we cannot know
# what Stat was used, we can do nothing here.
data$width <- data$width %||%
params$width %||% (resolution(data$x, FALSE) * 0.9)
transform(data,
Expand Down
7 changes: 5 additions & 2 deletions R/geom-col.r
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,11 @@ GeomCol <- ggproto("GeomCol", GeomRect,
non_missing_aes = c("xmin", "xmax", "ymin", "ymax"),

setup_data = function(data, params) {
data$width <- data$width %||%
params$width %||% (resolution(data$x, FALSE) * 0.9)
# To ensure bar widths are constant within a PANEL, `width` is not allowed
# to be mapped to data. So, even if the `data` already contains `width`, it
# should be ignored except when the width is calculated by Stat. But, as
# geom_col() uses stat_identity() fixedly, this is not the case.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wanted to point out that if somebody writes code such as stat_bin(geom = "col") they could arrive here with a calculated width column, but that would probably be considered inappropriate use then. May be worth a mention in the comment, for the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

data$width <- params$width %||% (resolution(data$x, FALSE) * 0.9)
transform(data,
ymin = pmin(y, 0), ymax = pmax(y, 0),
xmin = x - width / 2, xmax = x + width / 2, width = NULL
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-geom-col.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,12 @@ test_that("geom_col removes columns with parts outside the plot limits", {
"Removed 1 rows containing missing values"
)
})

test_that("geom_col() ignores width", {
dat <- data_frame(x = c(1, 2, 3))


p <- ggplot(dat, aes(x, x, width = x)) + geom_col()
d <- layer_data(p)
expect_equal(d$xmax - d$xmin, c(0.9, 0.9, 0.9))
})