ggplot2 icon indicating copy to clipboard operation
ggplot2 copied to clipboard

point is misplaced if width = 0 is used in position_dodge2 with geom_point

Open joelgautschi opened this issue 4 years ago • 4 comments

If position_dodge2(width = 0) is used with the position argument in geom_point() (one?) is wrongly placed horizontally (i. e., wrong 'column' for discrete values). This only seems to be an issue if width = 0. If position_dodge2() is not intended to be used with geom_point() a warning would be helpful.

library(tidyverse)

df <- tribble(~dimension, ~group, ~value,
              "dim 1", "A", 0.6,
              "dim 1", "B", 0.7,
              "dim 2", "A", 0.4,
              "dim 2", "B", 0.5)

plot <- ggplot(data = df,
               mapping = aes(x = dimension,
                             y = value,
                             color = group))  

# point in wrong column (point of 'dim 1' 'B' appears in column of 'dim 2'
plot +  geom_point(
             position = position_dodge2(width = 0))


# correct column
plot + geom_point(
  position = position_dodge2(width = 0.5))

Created on 2021-01-26 by the reprex package (v0.3.0)

joelgautschi avatar Jan 26 '21 11:01 joelgautschi

Thanks. So, maybe position_dodge2() should raise an error when width <= 0?

yutannihilation avatar Jan 27 '21 00:01 yutannihilation

I think only width<0 should raise an error.

To fix the bug need to rewrite a condition inside the find overlaps function. Here: https://github.com/tidyverse/ggplot2/blob/7ddb6d91513d3bfff43e2cee916eb819d4772a3e/R/position-dodge2.r#L141

There are two solutions, I think the first one is better.

  1. Fixing it by checking xmin of the next object is strictly greater than xmax of the previous object.
    if (is.na(df$xmin[i]) || is.na(df$xmax[i - 1]) || df$xmin[i] > df$xmax[i - 1]) {
  1. Check additionally equal case that the width is more than zero.
    if (is.na(df$xmin[i]) || is.na(df$xmax[i - 1]) || df$xmin[i] > df$xmax[i - 1] ||
        (df$xmin[i] == df$xmax[i - 1] && df$xmin[i] != df$xmax[i])) {

javlon avatar Aug 09 '22 19:08 javlon

I figured out that PR #4936 doesn't work correctly for one of the examples (diamonds) from the documentation.

I have tested the second solution (https://github.com/tidyverse/ggplot2/issues/4327#issuecomment-1209805927). It fixed the new bug and the previous one. ~~I have added this solution with PR #4944. (I'm not sure if it is ok or not)~~

javlon avatar Aug 13 '22 20:08 javlon

@javlon Sorry, it was my fault that I didn't notice the issue. Thanks for catching.

Could you just revert the change in a different PR (hopefully with a test, but it's optional)? Your first solution was simple enough so I thought it's worth adding, but now I'm not confident that there's no corner case that df$xmin[i] == df$xmax[i - 1] && df$xmin[i] != df$xmax[i] doesn't catch well.

(I still think this should be solved by just making it error because dodging with zero width sounds meaningless...)

yutannihilation avatar Aug 14 '22 01:08 yutannihilation