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

Feature Request: Upset axis as a guide #17

Open
teunbrand opened this issue Jun 8, 2020 · 4 comments
Open

Feature Request: Upset axis as a guide #17

teunbrand opened this issue Jun 8, 2020 · 4 comments

Comments

@teunbrand
Copy link

Hi there,

I was trying to make an upset plot when I noticed that the axis inherits the aspect ratio from the global theme in a weird way, reprex below:

library(ggplot2)
library(tidyverse)
library(ggupset)

theme_set(theme_gray())
theme_update(aspect.ratio = 1)

tidy_movies %>%
  distinct(title, year, length, .keep_all=TRUE) %>%
  ggplot(aes(x=Genres)) +
  geom_bar() +
  scale_x_upset(n_intersections = 20)
#> Warning: Removed 100 rows containing non-finite values (stat_count).

Created on 2020-06-08 by the reprex package (v0.3.0)

This could probably be fixed by not having the axis act on a global aspect ratio, but perhaps the nicer way of dealing with this is to implement the axis as a guide instead.

As far as I follow the ggplot2 github, it seems they are planning on a rewrite of the guide system. So, it may not be worth it to implement one now but it might be nice to consider in the future. It will probably make it easier to implement #2 too. I have some understanding of how the guide system works in current ggplot2, so I could try to draft a PR if I can find the time.

Thanks for considering!

@const-ae
Copy link
Owner

const-ae commented Jun 8, 2020

Hey, thank you for raising the issue.
You are of course right: the way the axis currently inherits the aspect ratio is bad. I guess the easiest way to solve is to make sure that the global aspect.ratio is explicitly ignored somewhere here.
On the other hand, if rewriting the package to use the guide system flexible would avoid some of the hacks like extending CoordTrans, this would be amazing. However, if the implementation on ggplot2 site is still in flux, I would wait with such an effort.

Is there some ggplot2 discussion/issue that's is worth reading on this?
Nonetheless, if you want to give it a try and start a PR, I am always happy to merge as long as it fixes a bug or makes the package more easily maintainable in the future :)

@teunbrand
Copy link
Author

It think the relevant issue is probably here, but I've seen it mentioned elsewhere too. More in-depth discussion on the current position guide system can be found here.

I hadn't considered that the CoordTrans is extended to make the upset plots work, which makes things a bit more complicated because I noticed that these don't work with the guide system nicely (yet). It is mentioned here.

I agree that probably from an efficiency viewpoint it would be best to delay large changes until after the switch to the ggproto system.

@teunbrand
Copy link
Author

Hi Constantin,

I made a draft in this gist of the combmatrix implemented as an axis guide. It's by no means very polished yet, the main issue being that the guide has no control over the size of plot elements besides the width (y-axis) or height (x-axis) of itself. This means that the current state of things is that you'd likely have to reserve some space for the text manually. If you could have a quick glance and see if this is something you might want for ggupset, that'd be great :) If you feel it isn't a great fit or downsides outweigh upsides, that is fine too (no hard feelings!).

Best wishes,
Teun

Below a few examples:

library(ggplot2)
library(ggupset)
library(dplyr)
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
source("https://gist.githubusercontent.com/teunbrand/16bfdbb48872c5d06a1cfb0c023ee613/raw/ef32fa6c59ccb1f0f438a953d6ec875607893c5d/guide_axis_combmatrix.R")

list2char <- function(x, sep = "-") {
  vapply(x, function(y) paste0(sort(y), collapse = sep), character(1))
}

g <- tidy_movies %>%
  distinct(title, year, length, .keep_all = TRUE) %>%
  ggplot(aes(x = list2char(Genres))) +
    geom_bar() +
    guides(x = guide_axis_combmatrix()) +
    theme(axis.text.x = element_text(hjust = 1, vjust = 0.5),
          axis.text.y = element_text(margin = margin(r = 2.2, l = 20)))
g

Works as one might expect with facetting:

g + facet_grid(~ grepl("Action", list2char(Genres)), 
               scales = "free_x", space = "free_x") +
  theme(panel.spacing.x = unit(60, "pt"))

Created on 2021-06-29 by the reprex package (v1.0.0)

@const-ae
Copy link
Owner

const-ae commented Jul 1, 2021

Thanks for the great example. There are certainly some upsides to using guides for the comb_matrix, it seems they fit more naturally into the ggplot framework than the CoordTrans system.

On the other hand, if I correctly understand they are not a 100% drop-in replacement for the current approach, so I think they could be added as a new (additional) feature but not as a replacement of the current system. What do you think about that?

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

No branches or pull requests

2 participants