Closed grantmcdermott closed 7 months ago
Grant, really cool, thanks for the work on this! I just want to comment on two aspects that we had visited before:
Layout: In my first attempt to add both faceting and categorical variables https://github.com/grantmcdermott/plot2/pull/12, I had just added an argument mfrow
so that we could set mfrow = TRUE
or mfrow = c(1, 4)
etc. Maybe this would be feasible for letting the users decide whether they want square layouts or not.
Axes: It would be good to separate out the functions for drawing the panels and drawing the axes now. This might let us add the support for categorical variables from https://github.com/grantmcdermott/plot2/pull/12 that we had further discussed in https://github.com/grantmcdermott/plot2/issues/2. My suggestion was that there should be workhorse functions a la plot2_xclass_yclass_plottype(x, y, by, axes = TRUE, ...)
.
From reading your description above (but not studying the code in detail), I thought that it might maybe help to have accompanying functions axis2_xlass_yclass_plottype(...)
that could be inserted into the workflow for drawing the axes?
Okay, I haven't managed to squeeze in everything. In particular, we'll need to address the single facet row soon and some of the other things you mention @zeileis. (Shouldn't be too hard to implement technically; we just need to decide on the UI options we want.) But as base for facet support in plot2—something that we can continue building on—I think that this PR is now ready to review and merge.
Quite chuffed with how this has come together TBH. Here's one more example, demonstrating the add functionality works with facets.
pkgload::load_all("~/Documents/Projects/plot2")
#> ℹ Loading plot2
data("penguins", package = "palmerpenguins")
penguins = subset(penguins, !is.na(body_mass_g) & !is.na(species))
# Predict body mass with interactions to match facet-by combo further below
mod = lm(body_mass_g ~ flipper_length_mm * species * sex, data = penguins)
# Add predictions to dataset
penguins = cbind(penguins, predict(mod, newdata = penguins, interval = "confidence"))
# Original obs
with(
penguins,
plot2(
x = flipper_length_mm, y = body_mass_g,
by = sex, facet = species,
palette = "dark2", fill = "by",
grid = TRUE, frame = FALSE,
main = "Penguins"
)
)
# Add model predictions
with(
penguins,
plot2(
x = flipper_length_mm, y = fit, ymin = lwr, ymax = upr,
by = sex, facet = species,
type = "ribbon",
palette = "dark2",
add = TRUE
)
)
Created on 2023-12-01 with reprex v2.0.2
I can`t look at the code, but this looks absolutely amazing. Well done!
Personally, I would avoid putting too much burden on the formula. Faceting does feel like a different functionality which should mostly (exclusively?) be called with its own argument. Also, focusing on the facet
argument eventually allow us to do more complex things like multiple variables with a two-sided formula indicating rows and columns.
The mfrow
feature seems important and powerful. Is that name too generic? Will people think it does something else, like allow multiple plot2()
calls? If it only applies to facet
, maybe it should be facet_mfrow
. I don't have a strong view...
Overall, very very nice stuff!
Okay, let's merge this as-is then and tackle the remaining facet questions in standalone issue(s). I'll copy them across shortly.
Closes #79.
Description
This PR introduces facet support to plot2. While user-facing changes are minimal—essentially all running through a new "facet" argument—it has required a fairly major refactoring of internal code to correctly interface with the existing "by" grouping code logic. At a high level, this is what plot2 does internally now:
par(mfrow)
to draw the axes, grid, frames and sub-titles of the individual facets. Do not add any interior plot elements yet. We're just setting the facet windows for now.par(mfg)
to make sure that we do this in the correct order.)This internal code refactoring has broken a few tests. Some are just to do with plotting order—which elements are plotted on top of which—so not a serious blocker. A potentially more serious issue is that manually configuring a grid of plots viaUPDATE: Fixed.par(mfrow)
no longer works consistently. I'm pretty sanguine about this and view it as an acceptable tradeoff (see also #65).Examples
Created on 2023-11-29 with reprex v2.0.2
Chores for this PR
plot2(..., add = TRUE)
doesn't work perfectly because of some internal adjustments for facet titles. In other words the added elements can be a little off in terms of y-axis spacing. UPDATE: fixed with introduction of global par2 options.Still to do / decide:
We don't necessarily have to resolve or roll these all into this PR. But here are some things that I think we ultimately need take a stance on w.r.t. facet support. EDIT: Moved to #90.