Skip to content

Allow NA values in position_dodge2 #4408

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

Merged
merged 3 commits into from
Apr 12, 2021
Merged

Allow NA values in position_dodge2 #4408

merged 3 commits into from
Apr 12, 2021

Conversation

thomasp85
Copy link
Member

Fix #2905

This fix catches rows with NA values and assigns them to a new group before they can cause error in the comparison

@thomasp85 thomasp85 added this to the ggplot2 3.3.4 milestone Apr 9, 2021
@thomasp85 thomasp85 requested a review from karawoo April 9, 2021 07:48
Copy link
Member

@karawoo karawoo left a comment

Choose a reason for hiding this comment

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

thanks!

@karawoo
Copy link
Member

karawoo commented Apr 9, 2021

Actually this could probably use a test as well

test_that("NA values are given their own group", {
  df <- data.frame(
    xmin = c(1, 2, NA, NA),
    xmax = c(1, 2, NA, NA)
  )
  expect_equal(find_x_overlaps(df), seq_len(4))
})

@thomasp85 thomasp85 merged commit 15ce4a9 into master Apr 12, 2021
@thomasp85 thomasp85 deleted the issue-2905-dodge2-NA branch April 12, 2021 09:28
sthagen added a commit to sthagen/tidyverse-ggplot2 that referenced this pull request Apr 12, 2021
Allow NA values in position_dodge2 (tidyverse#4408)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

position_dodge2 with NA values causes errors
2 participants