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

Dedicated draw_legend function #76

Merged
merged 14 commits into from
Oct 13, 2023
Merged

Dedicated draw_legend function #76

merged 14 commits into from
Oct 13, 2023

Conversation

grantmcdermott
Copy link
Owner

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
Copy link
Owner Author

@zeileis Do you mind reviewing this one for me?

@grantmcdermott
Copy link
Owner Author

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
Copy link
Collaborator

zeileis commented Sep 24, 2023

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: #2

@zeileis
Copy link
Collaborator

zeileis commented Sep 24, 2023

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
Copy link
Owner Author

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
Copy link
Collaborator

zeileis commented Oct 2, 2023

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
Copy link
Owner Author

grantmcdermott commented Oct 11, 2023

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
Copy link
Owner Author

grantmcdermott commented Oct 11, 2023

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
Copy link
Owner Author

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!

@grantmcdermott grantmcdermott merged commit db0beaf into main Oct 13, 2023
2 checks passed
@grantmcdermott grantmcdermott deleted the draw_legend branch October 13, 2023 03:12
@zeileis
Copy link
Collaborator

zeileis commented Oct 13, 2023

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
Copy link
Owner Author

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
Copy link
Collaborator

zeileis commented Oct 18, 2023

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?

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.

2 participants