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

Add facet support #83

Merged
merged 26 commits into from
Dec 4, 2023
Merged

Add facet support #83

merged 26 commits into from
Dec 4, 2023

Conversation

grantmcdermott
Copy link
Owner

@grantmcdermott grantmcdermott commented Nov 30, 2023

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:

  • Grab the "by" and "facet" variables (if any).
  • Open a new plot window, draw the legend (if "by" is not null) and add the other "common" plot elements like main title.
  • Loop over the facets (if any) and set 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.
  • Finally, draw the interior plot elements via a nested loop:
    • Outer loop: Split the data by the "by" grouping variable and loop over each group sub-sample.
    • Inner loop: Split the group sub-sample by the "facet" variable and loop over each facet window (manually toggling par(mfg) to make sure that we do this in the correct order.)
    • Draw all the interior plot elements (points, ribbons, etc.) for the by-facet combination designated by this stage of the nested loop.
    • Repeat for the remaining iterations.

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 via par(mfrow) no longer works consistently. I'm pretty sanguine about this and view it as an acceptable tradeoff (see also #65). UPDATE: Fixed.

Examples

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

data("penguins", package = "palmerpenguins")
penguins = subset(penguins, !is.na(body_mass_g) & !is.na(species))

par(pch = 19)

# basic faceting on its own
with(
  penguins,
  plot2(
    x = flipper_length_mm, y = body_mass_g,
    facet = species
  )
)

# fancier version with additional args (incl. interaction with separate by arg)
with(
  penguins,
  plot2(
    x = flipper_length_mm, y = body_mass_g,
    by = sex, facet = species,
    palette = "dark2",
    grid = TRUE, frame = FALSE,
    main = "Palmer Penguins"
  )
)

# facet = "by" (redundant but there if you want it)
with(
  penguins,
  plot2(
    x = flipper_length_mm, y = body_mass_g,
    by = species, facet = "by",
    grid = TRUE, frame = FALSE,
    main = "Palmer Penguins"
  )
)

# density example
with(
  penguins,
  plot2(
    x = body_mass_g, facet = species,
    type = "density",
    fill = "black",
    grid = TRUE, frame = FALSE,
    main = "Penguin body mass (g)"
  )
)

# and another one (incl. interaction with a separate by arg)
with(
  penguins,
  plot2(
    x = body_mass_g, by = sex, facet = species,
    type = "density",
    fill = "by", palette = "dark2",
    grid = TRUE, frame = FALSE,
    main = "Penguin body mass (g)"
  )
)

Created on 2023-11-29 with reprex v2.0.2

Chores for this PR

  • Right now, adding new elements to a faceted plot via 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.
  • Add/update tests.
  • Add facet examples to help file.
  • Update vignette.

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.

@grantmcdermott grantmcdermott changed the title Add facet support [WIP] Add facet support Nov 30, 2023
@zeileis
Copy link
Collaborator

zeileis commented Nov 30, 2023

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 #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 #12 that we had further discussed in #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?

@grantmcdermott grantmcdermott changed the title [WIP] Add facet support Add facet support Dec 1, 2023
@grantmcdermott grantmcdermott marked this pull request as ready for review December 2, 2023 00:04
@grantmcdermott
Copy link
Owner Author

grantmcdermott commented Dec 2, 2023

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

@vincentarelbundock
Copy link
Collaborator

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!

Merge branch 'main' into facets

# Conflicts:
#	README.md
#	docs/CITATION.md
#	docs/README.md
#	docs/freeze.rds
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-10.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-11.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-2.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-3.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-4.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-5.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-6.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-7.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-8.png
#	docs/man/plot2.markdown_strict_files/figure-markdown_strict/unnamed-chunk-1-9.png
#	docs/man/plot2.md
#	docs/vignettes/get_started.Rmd
#	docs/vignettes/get_started.markdown_strict_files/figure-markdown_strict/density_topright-1.png
#	docs/vignettes/get_started.markdown_strict_files/figure-markdown_strict/ribbon_pred-1.png
#	docs/vignettes/get_started.md
@grantmcdermott
Copy link
Owner Author

Okay, let's merge this as-is then and tackle the remaining facet questions in standalone issue(s). I'll copy them across shortly.

@grantmcdermott grantmcdermott merged commit 3c40a1e into main Dec 4, 2023
3 checks passed
@grantmcdermott grantmcdermott mentioned this pull request Dec 4, 2023
11 tasks
@grantmcdermott grantmcdermott deleted the facets branch December 4, 2023 21:24
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.

Facet support
3 participants