Skip to content

Better error message when geom_boxplot() gets group data with more than 1 row #3321

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
May 9, 2019

Conversation

paleolimbot
Copy link
Member

This is a PR to fix #3316, where group data with more than 1 row in GeomBoxplot$draw_layer() was causing an error that was difficult to diagnose. This may occur when using geom_boxplot(stat = "identity"), and is likely to occur if a user wants to draw several boxplots at specific locations on a plot in a boxplot/scatter mashup situation.

library(ggplot2)
ggplot(
  data.frame(x = "one value", y = 3, value = 4:6),
  aes(x, ymin = 0, lower = 1, middle = y, upper = value, ymax = 10)
) + 
  geom_boxplot(stat = "identity", position = "identity")
#> Error: Elements must equal the number of rows or 1

Copy link
Member

@clauswilke clauswilke left a comment

Choose a reason for hiding this comment

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

LGTM, except the one minor formatting issue I point out.

R/geom-boxplot.r Outdated
# this may occur when using geom_boxplot(stat = "identity")
if (nrow(data) != 1) {
stop(
"Can't draw more than one boxplot per group. Did you forget aes(group=...)?",
Copy link
Member

Choose a reason for hiding this comment

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

I think there should be a space before and after the = sign.

@clauswilke
Copy link
Member

Sorry, one more issue: Could you amend the latest commit message so it includes "Closes #3316" or "Fixes #3316"? This will allow github to automatically link the commit to the bug and it will close the bug upon merge.

Example: cfd4962

@paleolimbot paleolimbot force-pushed the issue-3316-boxplot-nrow branch from f94bdd9 to 8ee5f88 Compare May 9, 2019 18:24
@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #3321 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3321   +/-   ##
=======================================
  Coverage   78.94%   78.94%           
=======================================
  Files         171      171           
  Lines        6692     6692           
=======================================
  Hits         5283     5283           
  Misses       1409     1409
Impacted Files Coverage Δ
R/geom-boxplot.r 92.59% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b69172...8ee5f88. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented May 9, 2019

Codecov Report

Merging #3321 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3321   +/-   ##
=======================================
  Coverage   78.94%   78.94%           
=======================================
  Files         171      171           
  Lines        6692     6692           
=======================================
  Hits         5283     5283           
  Misses       1409     1409
Impacted Files Coverage Δ
R/geom-boxplot.r 92.59% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b69172...8ee5f88. Read the comment docs.

@clauswilke
Copy link
Member

Did the force push undo the commit that fixed the spacing?

Also, one issue that may or may not be related: I'm seeing a lot of github activity with strange time stamps (events occurring in the future, or in the wrong order). Is your machine set up to use local time? It's generally best to set up the machine time to UTC and then tell the machine what time zone you're in, rather than running it on local time.

@paleolimbot
Copy link
Member Author

It did! I googled how to amend a commit message and it backfired. Will fix.

@paleolimbot
Copy link
Member Author

I'm glad you're seeing the datetime issue as well...I thought I was going nuts. I think it happens with all my github activity from my laptop, which is why some stuff is out of order. Hopefully tomorrow I will be back to living in the present.

@clauswilke clauswilke merged commit 7364e13 into tidyverse:master May 9, 2019
@lock
Copy link

lock bot commented Nov 5, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Nov 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

geom_boxplot() with data rows >1 and stat = "identity" fails
2 participants