grantmcdermott / tinyplot

Lightweight extension of the base R graphics system
https://grantmcdermott.com/tinyplot
Apache License 2.0
226 stars 7 forks source link

Dedicated draw_legend function #76

Closed grantmcdermott closed 11 months ago

grantmcdermott commented 1 year ago

This PR breaks the legend drawing component into a separate, dedicated function: draw_legend().

In addition to simplifying the main plot2() code, it has the virtue that it should enable dual legends down the road (e.g. for "bubble" charts as per discussion in #50).

I also fixed some minor issues with the old legend creation process (mostly related to suboptimal plot margins), which is why we have a few updated snapshots too.

grantmcdermott commented 1 year ago

@zeileis Do you mind reviewing this one for me?

grantmcdermott commented 1 year ago

Sorry to nag @zeileis, but do you think you’ll have time to look at this?

I don’t mean to rush you (no obligations on your side!). But I’m hoping to push out the next plot2 release sooner rather than later, and would like to get this internal change sorted before adding one or two more features. If you don’t have time—again, no pressure—then I’ll probably merge as-is, since it doesn’t affect any user-facing syntax.

zeileis commented 1 year ago

Apologies for being so slow (again). I had taken a quick look and did not really understand exactly how draw_legend() is supposed to work. I tried to look again today and came to the same conclusion. I mean it's clear to me that you took out a portion of plot2() and put it into a separate function and that's fine. But how should the new function be used in the future and what does it enable? Always just internally or also by the user? Do you have any examples that would illustrate the purpose?

I would specifically be interested in how the function might facilitate (or not) faceting and the plot2_x_y_type() idea from: https://github.com/grantmcdermott/plot2/issues/2

zeileis commented 1 year ago

Small side note because I saw utils::modifyList() being used in the code. It is better to import such a function in the NAMESPACE and then use it just as modifyList() (without utils::) as this avoids the overhead of search utils' namespace again and again.

I wasn't aware of this inefficiency of pkg::fun() constructs until recently because R-core is currently discussing the issue.

grantmcdermott commented 1 year ago

Thanks @zeileis.

Apologies for not making the use-cases clearer. But, yes, my primary goal in moving it to a stand-alone function is make it easy re-use in situations like those required by plot_x_y_type. Another canonical example that I had in mind would be for bubble charts, where you often need to produce two separate legends to handle both size and color (c.f. these ggplot2 examples).

I don't intend for draw_legend to be user facing, since it requires going through the plot.new -> plot.window -> ... workflow. So it will mostly be for internal purposes and ease of re-using our legend-creation logic.

I'm typing this all on my phone, but will carve out some time during the week to provide some clearer examples. Thanks too for the namespace tip: TIL!

zeileis commented 1 year ago

Thanks for the explanations, Grant @grantmcdermott ! Bubble charts are a good motivation for this. I look forward to some examples - but no pressure from me :-)

grantmcdermott commented 11 months ago

I'm not sure how compelling this example will be as a manual implementation. But here is the sort of functionality that I envisage having a separate draw_legend function will enable.

devtools::load_all("~/Documents/Projects/plot2/")
#> ℹ Loading plot2

# utilty rescaling function for bubble plot legend
rscl = function (x) {
  if (length(x) > 5) {
    x = pretty(x, n = 5)
  } else {
    x = sort(x)
  }
  x = x[x > 0]
  from = range(x)
  to = c(1, 2.5)
  out = (x - from[1])/diff(from) * diff(to) + to[1]
  names(out) = x
  return(out)
}

# main plot
plot2(
  Sepal.Length ~ Petal.Length | Species, iris,
  cex = rscl(iris$Petal.Width),
  grid = TRUE, legend = "topright!" 
)

# now add the size legend
l = rscl(iris$Petal.Width)
draw_legend(
  "bottomright!",
  legend.args = list(),
  by_dep = "Petal.Width",
  lgnd_labs = names(l),
  cex  = l,
  type = "p",
  pch = 1,
  col = "black",
  new_plot = FALSE
)

Created on 2023-10-10 with reprex v2.0.2

Obviously, we need to figure out positioning/alignment of two legends... and any dedicated type = "bubble" plot type that we support down the line would have to handle this automatically. (Again, I don't think users should be calling draw_legend manually except perhaps at their own risk.)

But IMO those tweaks can come down the line when they are implemented for the different chart types. So having draw_legend as a separate function doesn't create any downside in the interim, at least. And I think it will make it easier to compose multiple legends internally once we reach that point.

grantmcdermott commented 11 months ago

Another example, this time for faceted plots. I'll certainly grant that this manual implementation is pretty clunky. But I think it shows a path towards how we could actually implement facets internally... partly by using a dedicated draw_legend function to help draw a new plot window and set the overall legend position first, and then add the faceted plots afterwards. (Or, at least, how I've been thinking about it.)

devtools::load_all("~/Documents/Projects/plot2/")
#> ℹ Loading plot2
data("penguins", package = "palmerpenguins")

par(pch = 19)

# Draw (empty) new plot with legend first 
draw_legend(
  "bottom!",
  legend.args = list(),
  by_dep = "sex",
  lgnd_labs = levels(penguins$sex),
  type = "p",
  pch = 19,
  cex  = 1,
  col = palette()[1:2]
)

# Now set the facets
par(mfrow = c(1, 3))
# See: https://github.com/grantmcdermott/plot2/issues/65
par(mfg = c(1, 1))

# Finally, draw the individual plots.
## This next chunk is just a manual mock up of stuff that we would handle
## internally within plot2
penguins_split = split(penguins, penguins$species)
invisible(lapply(
  seq_along(penguins_split),
  \(i) plot2(
    body_mass_g ~ flipper_length_mm | sex, penguins_split[[i]], 
    frame = FALSE, grid = TRUE,
    legend = FALSE,
    main = names(penguins_split)[[i]],
    xlim = range(penguins$flipper_length_mm, na.rm = TRUE),
    ylim = range(penguins$body_mass_g, na.rm = TRUE)
    )
))

Created on 2023-10-10 with reprex v2.0.2

grantmcdermott commented 11 months ago

I'm going to go ahead and merge this, since I plan to dedicate some dev time over the next few days for additional features that build on this change. Hope that's okay!

zeileis commented 11 months ago

OK, thanks. I didn't have time until today due to heavy teaching on Tue/Wed/Thu.

Thanks also for the facets example, very useful. Do you also have thoughts on how to modify the par and the drawing of the axes in this example?

grantmcdermott commented 11 months ago

Not fully within the plot2 framework yet. I do have another mock-up here that tries to do some more sophisticated plotting of the axes and facet titles to avoid duplication. But even this example leaves some awkward spacing between the individual plots.

One open question is whether to go through par(mfrow)/par(mfcol), which is what I originally thought... Versus going through layout(). I haven't used the latter enough to have a sense of its advantages over the former.

zeileis commented 11 months ago

So far when I implemented things like this I used mfrow/mfcol plus mar/oma. For example for multivariate plot.zoo which is mimicking plot.ts plus some extensions. This collapses many of the margins, omits certain axes/annotation, while some of the annotation is drawn in the outer margins.

I haven't worked much with layout() either but don't see many advantages if we stick to the ideas above that annotations are drawn in the outer margins.

What might be an advantage - I haven't tried - is that layout() should be flexible enough to set up suitable panels for legend, annotation, etc. Have you ever tried to play around with this?