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

add orientation and better compatible with coord_flip #104

Merged
merged 6 commits into from Aug 5, 2021

Conversation

xiangpin
Copy link
Contributor

@xiangpin xiangpin commented Aug 5, 2021

  • The ggplot2 has introduced orientation to control the orientation of the layer (either "x" or "y"). This is useful because some geometric layers can not change the "x" or "y" in the mapping, such as geom_density_ridges.
  • In addition, the layer can also use orientation to directly control the orientation of the layer, such as geom_col, geom_bar, geom_bar ... But, when it is also compatible with the coord_flip.

original

library(ggplot2)
library(ggsignif)
dat <- data.frame(
            Group = c("S1", "S1", "S2", "S2"),
            Sub = c("A", "B", "A", "B"),
             Value = c(3, 5, 7, 8)
            )
p1 <- ggplot(dat, mapping=aes(x=Group, y=Value)) +
             geom_col(aes(fill=Sub), position="dodge", width=0.5) + 
             geom_signif(y_position=c(5.3, 8.3), 
                                 xmin=c(0.8, 1.8), 
                                 xmax=c(1.2, 2.2),
                                 annotation=c("**", "NS"), 
                                 tip_length=0) +
             geom_signif(comparisons = list(c("S1", "S2")),
                                  y_position = 9.3, 
                                  tip_length = 0, 
                                  vjust = 0.2) + 
             scale_fill_manual(values = c("grey80", "grey20"))
p1

p1

Introducing orientation

p2 <- ggplot(dat, mapping=aes(y=Group, x=Value)) +
          geom_col(aes(fill=Sub), position="dodge", width=0.5, orientation="y") + 
          geom_signif(y_position=c(5.3, 8.3), 
                              xmin=c(0.8, 1.8), 
                              xmax=c(1.2, 2.2),
                              annotation=c("**", "NS"), 
                              tip_length=0, 
                              orientation="y") +
          geom_signif(comparisons = list(c("S1", "S2")),
                               y_position = 9.3, 
                               tip_length = 0, 
                               vjust = 0.2, 
                                orientation="y") + 
          scale_fill_manual(values = c("grey80", "grey20"))
p2

p2

Orignal + coord_flip

p3 <- p1 + coord_flip()
p3

p3

orientation + coord_flip

p4 <- p2 + coord_flip()
p4

p4

@IndrajeetPatil
Copy link
Collaborator

Thank you so much for working on this! This looks fantastic.

Just a couple of minor things:

  • Why are you adding a Makefile? Is it necessary?
  • Can you please make an entry in NEWS.md about this change?

Thanks

@const-ae
Copy link
Owner

const-ae commented Aug 5, 2021

@xiangpin, Wow this is really great, thank you for addressing this longstanding shortcoming.

@IndrajeetPatil, do you know what is going on with all the CI failures. Is that expected?

Can you please make an entry in NEWS.md about this change?

That's a good idea. In addition, I would suggest adding an example in the README.Rmd and also if possible some unit tests

@IndrajeetPatil
Copy link
Collaborator

IndrajeetPatil commented Aug 5, 2021

Some of the tests need to be updated because of this change. I can update them once this is merged.

@xiangpin
Copy link
Contributor Author

xiangpin commented Aug 5, 2021

Hello @IndrajeetPatil @const-ae

I have made some updates according to your suggestions.

  • The Makefile is useful for developers to build or check ... on the local computer since it can save time by collecting common commands. For example, make all can replace the R CMD build ..., R CMD check ... etc. in this file.
  • I have updated the NEWS.md, added an example to the README, and modified the unit test.

best wishes
xushuangbin

@IndrajeetPatil IndrajeetPatil merged commit 66688b1 into const-ae:master Aug 5, 2021
@IndrajeetPatil
Copy link
Collaborator

Excellent. Thanks once again for your contribution! :)

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.

None yet

3 participants