Skip to content
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

violin plots are rendered with long tails in current stable GGPLOT2 #1700

Closed
nbafrank opened this issue Aug 7, 2016 · 17 comments
Closed

violin plots are rendered with long tails in current stable GGPLOT2 #1700

nbafrank opened this issue Aug 7, 2016 · 17 comments
Assignees
Labels
bug

Comments

@nbafrank
Copy link

@nbafrank nbafrank commented Aug 7, 2016

Dear Hadley,

I and others have notice that the current version of GGPLOT2 renders violin plots with long tails at the upper end unlike previous versions.

Is there any way to revert back to the old rendering?

Thanks,

Francesco

@thomasp85
Copy link
Member

@thomasp85 thomasp85 commented Aug 12, 2016

Can you provide a reproducible example?

@thomasp85 thomasp85 added the reprex label Aug 12, 2016
@nbafrank
Copy link
Author

@nbafrank nbafrank commented Aug 24, 2016

res <- data.frame(a = c(1:100),b=c(rep(0,50),rep(1,50)))
ggplot(res,aes(y=a,x=factor(b))) + geom_violin(trim=F)

When you plot the violins, the tails go to reach the very end of the plot (both ends). Prior to version 2.0.0 (I believe) this was not occurring. It is solely an aesthetic change but I was wondering if there's an option to make it look like it used to be.

Thanks,

FV

@nbafrank
Copy link
Author

@nbafrank nbafrank commented Aug 31, 2016

Let me know if you need anything else =)

@hadley
Copy link
Member

@hadley hadley commented Sep 15, 2016

Use trim = TRUE?

@nbafrank
Copy link
Author

@nbafrank nbafrank commented Sep 17, 2016

so that option will trim and cut the violin. I want a full violin without the ends going all the way to the top like it used to be in the past. Is there a hidden option or you have changed some internal graphic component in an unfixable way? Thanks,FV

@hadley
Copy link
Member

@hadley hadley commented Sep 18, 2016

Can you please carefully explain the previous behaviour and the current behaviour? I don't understand what you want.

@wch
Copy link
Member

@wch wch commented Sep 18, 2016

@nbafrank in the future screenshots would help. You can use devtools::install_version to install an old version of a package.

This is the plot with 1.0.1:

violin-1 0 1

And the current dev version:

violin-dev

@nbafrank
Copy link
Author

@nbafrank nbafrank commented Sep 18, 2016

Thanks WCH, I did not know about devtools::install_version =) hope this is clear.

@hadley
Copy link
Member

@hadley hadley commented Sep 23, 2016

@wch do you know what caused the change?

@hadley hadley added bug and removed reprex labels Sep 23, 2016
@wch
Copy link
Member

@wch wch commented Sep 23, 2016

Nope, sorry. It looks like the y-range is now determined by the raw data instead of the transformed data.

@hadley
Copy link
Member

@hadley hadley commented Sep 23, 2016

I guess someone will have to git bisect this

@wch
Copy link
Member

@wch wch commented Sep 23, 2016

@hadley it's your fault: d58b00a

@hadley
Copy link
Member

@hadley hadley commented Sep 23, 2016

The reason I have an intern is to fix my mistakes...

@thomasp85
Copy link
Member

@thomasp85 thomasp85 commented Sep 23, 2016

You're going to be in so mush trouble in 7 days

@nbafrank
Copy link
Author

@nbafrank nbafrank commented Sep 24, 2016

@hadley @thomasp85 @wch thanks for your help guys. Looking forward to the new version. Maybe make it an option for both styles?

@thomasp85
Copy link
Member

@thomasp85 thomasp85 commented Sep 26, 2016

@hadley thinking about this, there is actually a value in the new approach even though it can look a bit odd, in that it communicate the range of the data clearly - do you want a complete roll-back to the old style (without reemerging the bug that caused the change) or what do you think..?

@hadley
Copy link
Member

@hadley hadley commented Sep 26, 2016

The current dev version doesn't look right because it's truncated to early on one end and too late on another - I think it's probably just that the ranges need to be calculated per group, not overall.

thomasp85 added a commit that referenced this issue Sep 28, 2016
Fixes #1700

* Expand ydensity range for nicer violin plots

* Add news bullet

* Update unit tests
@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants