Skip to content

make preserve = "single" more robust in position_dodge(). #2813

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

Conversation

clauswilke
Copy link
Member

This is an attempt at fixing #2801. Not sure this is ready to merge yet. One problem I see is that the violins are always moved to the left of the reference point, even if they should be moved to the right. Not sure how position_dodge() could know how to do this right.

Also, does a similar problem occur in position_dodge2()?

library(ggplot2)
p <- ggplot(mtcars, aes(factor(cyl), mpg))

# effect on geom_boxplot()
p + geom_boxplot(aes(fill = factor(vs)), position = "dodge")

p + geom_boxplot(aes(fill = factor(vs)), position = position_dodge(preserve = "single"))

# effect on geom_violin()
p + geom_violin(aes(fill = factor(vs)), position = "dodge")

p + geom_violin(aes(fill = factor(vs)), position = position_dodge(preserve = "single"))

Created on 2018-08-08 by the reprex package (v0.2.0).

@clauswilke

This comment has been minimized.

@lionel-

This comment has been minimized.

@clauswilke

This comment has been minimized.

@lionel-

This comment has been minimized.

@lionel-

This comment has been minimized.

@karawoo
Copy link
Member

karawoo commented Aug 8, 2018

One problem I see is that the violins are always moved to the left of the reference point, even if they should be moved to the right. Not sure how position_dodge() could know how to do this right.

Yeah this has been reported for bars in the past (#2076) and I don't see a way to fix it in position_dodge().

Also, does a similar problem occur in position_dodge2()?

position_dodge2() doesn't work at all for violins or dot plots, at the time I wrote it we decided to punt on making it compatible with those geoms.

@clauswilke

This comment has been minimized.

@clauswilke

This comment has been minimized.

@clauswilke clauswilke force-pushed the issue-2801-position-dodge-preserve-single branch from a5871f1 to be02e29 Compare August 22, 2018 03:40
@hadley
Copy link
Member

hadley commented Sep 12, 2018

I'd rather not make any more position_dodge() changes without thoroughly thinking them through. This might be a project for @thomasp85 to take on — IIRC position_dodge() and position_dodge2() are quite slow, and I think the algorithms are more elegantly expressed in C/C++.

@clauswilke
Copy link
Member Author

That's fine. Should I leave this PR open as a reminder or close? #2801 remains open.

@hadley
Copy link
Member

hadley commented Sep 12, 2018

Close, I think

@lock
Copy link

lock bot commented Mar 11, 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 Mar 11, 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.

4 participants