griffithlab / GenVisR

Genome data visualizations
Creative Commons Zero v1.0 Universal
209 stars 62 forks source link

ggplot2 v.2.2.0 breaks alignment #285

Closed alanocallaghan closed 7 years ago

alanocallaghan commented 7 years ago

I don't have time to post reproducible examples right now (will do this evening) but it seems like some changes to ggplot 2.2.0's handling of legends has broken the stability of the alignment code.

zlskidmore commented 7 years ago

@Alanocallaghan Thanks for the heads up!

It's not immediately clear to me what the issue could be from the ggplot2 news page https://github.com/tidyverse/ggplot2/blob/master/NEWS.md, setting a consistent theme for all plots (theme_void())seems to help, but does not actually alleviate the issue, This will require further investigation on my part.

This code will reproduce the issue:

install.packages("ggplot2")
library(GenVisR)
waterfall(brcaMAF, fileType = "MAF", mainRecurCutoff = 0.05)
alanocallaghan commented 7 years ago

I'm not sure if it's related but I used your technique of matching grob width (in waterfall_align.R) in a separate function recently, and I found that I had to use all widths, rather than just 2:5. However I didn't investigate what exactly this was setting so I don't know if it applies here

alanocallaghan commented 7 years ago

Hi @zlskidmore, I just got around to testing that. If you get rid of the subsetting (2:5) in all of the width getting and setting in waterfall_align.R, it seems to fix the bug. Again not sure what else this may affect but it does solve this issue.

zlskidmore commented 7 years ago

Thanks @Alanocallaghan! sorry for the delay I just got back from a conference, i'll implement your solution tomorrow and test to see if it effect's anything else. I think it should be okay, my only concern is if it will work with older ggplot2's but worst case scenario i'll just require a new version.

alanocallaghan commented 7 years ago

No problem, I had not had the chance to have a look either until this evening. I was working using ggplot 2.1 from MRAN while I couldn't get around the bug in any case. Cheers

zlskidmore commented 7 years ago

Looks like this solution will work for all but the compIdent function, keeping this open until i can fix that