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

Make legend glyphs configurable #3145

Closed
clauswilke opened this issue Feb 15, 2019 · 21 comments
Closed

Make legend glyphs configurable #3145

clauswilke opened this issue Feb 15, 2019 · 21 comments
Labels
Milestone

Comments

@clauswilke
Copy link
Member

@clauswilke clauswilke commented Feb 15, 2019

Currently all geoms have a hard-coded legend glyph that can't be changed without some knowledge about the inner workings of ggplot2. It would be helpful if the glyphs could be configurable. Associated with such a change, we might want to provide a broader array of different glyphs. One example is the stylized time series glyph of PR #3124.

It seems to me the easiest way to achieve this modification is to provide an additional parameter draw_key to the constructor of each geom. The downside is that it requires a modification of each individual geom_*() and will not automatically propagate to extension packages. The upside is that the modification required is rather minor. Example implementation follows.

library(ggplot2)

# alternative key glyph
draw_key_timeseries <- function(data, params, size) {
  data$linetype[is.na(data$linetype)] <- 0

  grid::linesGrob(
    x = c(0, 0.4, 0.6, 1),
    y = c(0.1, 0.6, 0.4, 0.9),
    gp = grid::gpar(
      col = scales::alpha(data$colour, data$alpha),
      lwd = data$size * .pt,
      lty = data$linetype,
      lineend = "butt"
    )
  )
}

# helper function
set_draw_key <- function(geom, draw_key = NULL) {
  if (is.null(draw_key)) {
    return(geom)
  }
  if (is.character(draw_key)) {
    draw_key <- paste0("draw_key_", draw_key)
  }
  draw_key <- match.fun(draw_key)
  
  ggproto("", geom, draw_key = draw_key)
}

# version of `geom_line()` with modifiable draw key
geom_line2 <- function(mapping = NULL, data = NULL, stat = "identity",
                       position = "identity", na.rm = FALSE,
                       show.legend = NA, inherit.aes = TRUE, draw_key = NULL,
                       ...) {
  layer(
    data = data,
    mapping = mapping,
    stat = stat,
    geom = set_draw_key(GeomLine, draw_key),
    position = position,
    show.legend = show.legend,
    inherit.aes = inherit.aes,
    params = list(
      na.rm = na.rm,
      ...
    )
  )
}

# usage
base <- ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y))
base + geom_line2(aes(color = "default"))

base + geom_line2(aes(color = "modified"), draw_key = "timeseries")

base + geom_line2(aes(color = "modified"), draw_key = draw_key_timeseries)

Created on 2019-02-15 by the reprex package (v0.2.1)

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Feb 15, 2019

One note: If we go this route, we may want to make the draw_key_*() functions more robust to missing aesthetics.

base + geom_line2(aes(color = "modified"), draw_key = "vpath")

base + geom_line2(aes(color = "modified"), draw_key = "abline")

base + geom_line2(aes(color = "modified"), draw_key = "point")
#> Error in check.length(gparname): 'gpar' element 'fontsize' must not be length 0

base + geom_line2(aes(color = "modified"), draw_key = "polygon")
#> Error in check.length("fill"): 'gpar' element 'fill' must not be length 0

@jemus42
Copy link
Contributor

@jemus42 jemus42 commented Feb 26, 2019

I often find myself wanting to have the legend glyphs just solid colors, for example in cases where I'm plotting points with different colors and no other geoms – in that case I would prefer the legend to simply indicate the different colors instead of indicating colors and a geometric shape that is not required to be identifiable.

If this proposed change is implemented, would that mean I could choose to override legend keys with "just give me a solid box" easily?

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Feb 26, 2019

yes.

@baptiste
Copy link
Contributor

@baptiste baptiste commented Mar 16, 2019

I've seen plots where the legend reproduced a miniature trace of the data it's referring to (e.g for time-series); it worked quite well.

Kind of related to this, I've come to believe that guides could be based on a trimmed down grammar of graphics. Basically, a guide would become a kind of mini ggplot itself.

https://gist.github.com/baptiste/960186a01ca3c48b112f4a15fe013884

It might be impractical in terms of code re-use, I'm not sure; obviously for speed and other reasons one would want a drastically-simplified framework (no grids, strips, scale training, stats, transformations, etc. — only the already-processed data would be passed, and a subset at that).

The output of ggplot_build contains data that could be relatively easily processed by a simplified plotting routine, but that still contains axes, theming, facetting, and other customisations.

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 17, 2019

@baptiste I think what you're bringing up is a separate issue. This specific issue is meant to make a small modification to the existing guide code. What you're proposing would require a complete overhaul of that code. I'm not opposed to that in principle, but I think it deserves its own issue.

@hadley @thomasp85 @yutannihilation Could you take a quick look at my functions set_draw_key() and geom_line2() at the top of this issue and let me know if you have any concerns about this approach to making legend keys configurable?

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Mar 18, 2019

Looks OK.

But, I'm wondering if it is really what we want to specify draw_key on individual geoms. For example, do we really need different lines for different line geoms in the plot below?

library(ggplot2)

# alternative key glyph
draw_key_timeseries <- function(data, params, size) {
  data$linetype[is.na(data$linetype)] <- 0
  
  grid::linesGrob(
    x = c(0, 0.4, 0.6, 1),
    y = c(0.1, 0.6, 0.4, 0.9),
    gp = grid::gpar(
      col = scales::alpha(data$colour, data$alpha),
      lwd = data$size * .pt,
      lty = data$linetype,
      lineend = "butt"
    )
  )
}

# helper function
set_draw_key <- function(geom, draw_key = NULL) {
  if (is.null(draw_key)) {
    return(geom)
  }
  if (is.character(draw_key)) {
    draw_key <- paste0("draw_key_", draw_key)
  }
  draw_key <- match.fun(draw_key)
  
  ggproto("", geom, draw_key = draw_key)
}

# version of `geom_line()` with modifiable draw key
geom_line2 <- function(mapping = NULL, data = NULL, stat = "identity",
                       position = "identity", na.rm = FALSE,
                       show.legend = NA, inherit.aes = TRUE, draw_key = NULL,
                       ...) {
  layer(
    data = data,
    mapping = mapping,
    stat = stat,
    geom = set_draw_key(GeomLine, draw_key),
    position = position,
    show.legend = show.legend,
    inherit.aes = inherit.aes,
    params = list(
      na.rm = na.rm,
      ...
    )
  )
}
# usage
base <- ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y))

base +
  geom_line(aes(color = "base")) +
  geom_line2(aes(color = "modified"), draw_key = "timeseries")

Created on 2019-03-18 by the reprex package (v0.2.1.9000)

One idea is to have a list of draw keys on ggplot_global and set_draw_key("line", draw_key_timeseries) will override default draw_key() all line-alike geoms. Probably, something like get_draw_key(layer$geom$legend_type()) would be called here:

draw_key = layer$geom$draw_key,

(I'm posting this just for discussion. I don't have any strong opinions yet)

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Mar 18, 2019

Sorry, I mistakenly closed the issue...

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 18, 2019

I am not too worried about having to specify a draw key multiple times if somebody uses the same geom multiple times in a plot. That's not any different than, for example, setting the same scale name in all scales (e.g. color, fill, shape) to ensure the legends merge correctly.

We could talk about making draw keys globally configurable, but I feel that's a secondary issue that we can provide in addition to having localized customization, not instead.

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 18, 2019

Actually, changing draw keys globally is trivially possible by modifying the geom. We could add a convenience function that encapsulates this modification if we wanted to.

library(ggplot2)

# alternative key glyph
draw_key_timeseries <- function(data, params, size) {
  data$linetype[is.na(data$linetype)] <- 0
  
  grid::linesGrob(
    x = c(0, 0.4, 0.6, 1),
    y = c(0.1, 0.6, 0.4, 0.9),
    gp = grid::gpar(
      col = scales::alpha(data$colour, data$alpha),
      lwd = data$size * .pt,
      lty = data$linetype,
      lineend = "butt"
    )
  )
}
# change draw key globally
GeomLine$draw_key <- draw_key_timeseries

ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y)) +
  geom_line(aes(color = "base")) +
  geom_line(aes(y = 4-y, color = "modified"))

Created on 2019-03-17 by the reprex package (v0.2.1)

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Mar 18, 2019

We could talk about making draw keys globally configurable, but I feel that's a secondary issue that we can provide in addition to having localized customization, not instead.

That's convincing, thanks. Then, I have no concern here.

One thing, to be clear, what I was trying to see was not to set the draw_key of individual Geoms, but to override draw_key_*() (e.g. draw_key_line()) globally, so that it might be possible for themes to provide custom legends globally.

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 18, 2019

Actually, now that I think about it, maybe starting with just the function that modifies geoms globally is the way to go. It would be minimally invasive. (I realize I use the same function name set_draw_key() for a different purpose now.)

library(ggplot2)

# function to adjust draw key globally
set_draw_key <- function(geom, draw_key) {
  geom <- ggplot2:::check_subclass(geom, "Geom", env = parent.frame())

  if (is.character(draw_key)) {
    draw_key <- paste0("draw_key_", draw_key)
  }
  draw_key <- match.fun(draw_key)
  
  geom$draw_key <- draw_key
}

# alternative key glyph
draw_key_timeseries <- function(data, params, size) {
  data$linetype[is.na(data$linetype)] <- 0
  
  grid::linesGrob(
    x = c(0, 0.4, 0.6, 1),
    y = c(0.1, 0.6, 0.4, 0.9),
    gp = grid::gpar(
      col = scales::alpha(data$colour, data$alpha),
      lwd = data$size * .pt,
      lty = data$linetype,
      lineend = "butt"
    )
  )
}

set_draw_key("line", "timeseries")
ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y, color = "line")) + geom_line()

set_draw_key("line", "path")
ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y, color = "line")) + geom_line()

Created on 2019-03-17 by the reprex package (v0.2.1)

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Mar 18, 2019

Can we keep the default draw_key function so that we can reset by set_draw_key("line", NULL)?

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 18, 2019

Like so? Note that I changed the name of the function.

library(ggplot2)

# function to adjust draw key globally
set_guide_glyph <- function(geom, key_glyph = NULL) {
  geom <- ggplot2:::check_subclass(geom, "Geom", env = parent.frame())

  if (is.null(key_glyph)) {
    # recover previous key glyph if known
    if (is.null(geom$draw_key_default)) return()
    
    geom$draw_key <- geom$draw_key_default
  } else {
    # if default doesn't exist, save current draw key as default
    if (is.null(geom$draw_key_default)) {
      geom$draw_key_default <- geom$draw_key
    }
    
    if (is.character(key_glyph)) {
      key_glyph <- paste0("draw_key_", key_glyph)
    }
    key_glyph <- match.fun(key_glyph)
  
    geom$draw_key <- key_glyph
  }
}

# alternative key glyph
draw_key_timeseries <- function(data, params, size) {
  data$linetype[is.na(data$linetype)] <- 0
  
  grid::linesGrob(
    x = c(0, 0.4, 0.6, 1),
    y = c(0.1, 0.6, 0.4, 0.9),
    gp = grid::gpar(
      col = scales::alpha(data$colour, data$alpha),
      lwd = data$size * .pt,
      lty = data$linetype,
      lineend = "butt"
    )
  )
}

set_guide_glyph("line", "timeseries")
ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y, color = "line")) + geom_line()

set_guide_glyph("line")
ggplot(data.frame(x = 1:3, y = 3:1), aes(x, y, color = "line")) + geom_line()

Created on 2019-03-17 by the reprex package (v0.2.1)

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 18, 2019

One thing, to be clear, what I was trying to see was not to set the draw_key of individual Geoms, but to override draw_key_*() (e.g. draw_key_line()) globally, so that it might be possible for themes to provide custom legends globally.

I would be wary of that, due to potential unexpected side effects. Even with the system I just proposed you might run into trouble if you wanted to make a compound plot with patchwork and use different legend keys in the different component plots.

@yutannihilation
Copy link
Member

@yutannihilation yutannihilation commented Mar 18, 2019

Like so?

Yes! Looks nice.

I would be wary of that, due to potential unexpected side effects.

Agreed.

@hadley
Copy link
Member

@hadley hadley commented Mar 18, 2019

I'd prefer not to have more functions that modify the state of ggplot2 objects, because I think it generally makes code harder to reason about. If you think this function is useful enough to override that consideration, it should invisibly return the previous value, as described in https://principles.tidyverse.org/side-effect-soup.html#make-easy-to-undo.

@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Mar 18, 2019

@hadley Your statement mirrors my initial concerns. Let's not go that route then. People can always directly modify the geoms if they want to. I've done that a couple of times in my own code and I've always found it dissatisfying, because inevitably the modification needs to be undone later on for a different plot.

I realized yesterday that it may also be possible to move the draw_key argument into the layer function, where it then would be available right away to all geoms. I'll explore this option.

@clauswilke clauswilke added this to the ggplot2 3.2.0 milestone Apr 11, 2019
@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Apr 11, 2019

I'll try to get this done for the 3.2.0 release.

In addition to making the key glyph configurable, I'll also have to add meaningful aesthetics default settings to all the key drawing functions, in case they are used for geoms that don't provide some of the aesthetics. @dpseidel, at some point you did a survey of all the default settings the various geoms currently use. Could you remind me where I can find that?

@dpseidel
Copy link
Member

@dpseidel dpseidel commented Apr 11, 2019

clauswilke added a commit to clauswilke/ggplot2 that referenced this issue Apr 11, 2019
@clauswilke
Copy link
Member Author

@clauswilke clauswilke commented Apr 12, 2019

Thanks Dana!

@lock
Copy link

@lock lock bot commented Oct 23, 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 Oct 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants