Skip to content

WIP: S7 theme elements #6355

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

Open
wants to merge 43 commits into
base: main
Choose a base branch
from
Open

Conversation

teunbrand
Copy link
Collaborator

This PR aims to fix part of #6352.

In essence it converts all S3 element_*() functions and margin() to S7 classes.

As it stands, there are some backwards compatibility issues for extensions:

  • Testing wether an object is a particular type element with e.g. inherits(x, "element_blank") will no longer work. From 4.3.0 onwards, using inherits(x, element_blank) will work, and S7::S7_inherits(x, element_blank) should work. Alternatively, one could in theory use inherits(x, c("element_blank", "ggplot2::element_blank")) for backward compatibility, but it'd require a change in the extension.
  • Typical extraction with $, [ or [[ of properties will no longer work and extensions will have to use @. On ggplot2's end, we can provide methods for the typical generics, but we'd run into Weird S7/S3 interaction RConsortium/S7#390.
  • Extensions implementing subclasses like marquee::element_marquee() or ggtext::element_markdown() will also have to write their elements as S7 classes. I cannot see a good solution to have this be backwards compatible.

yjunechoe pushed a commit to yjunechoe/ggplot2 that referenced this pull request Mar 8, 2025
…6335)

* Document vector passing to position_nudge

* Put nudge_y into aes call

As discussed in tidyverse#6355.

* Update documentation

* redocument

---------

Co-authored-by: Teun van den Brand <[email protected]>
@teunbrand
Copy link
Collaborator Author

A few things to consider:

  • For the inherit(x, "element_blank") problem, we could consider tagging on an additional class to the S7 objects, without the ggplot2:: prefix.
  • If it turns out there is too much broken in reverse dependencies, we could retain S3 classes for a cycle, implement @.element so that extensions can adapt before we commit to S7 element classes.

thomasp85 and others added 15 commits April 8, 2025 08:30
Merge branch 'main' into rc/3.5.2

# Conflicts:
#	DESCRIPTION
#	NAMESPACE
#	R/aes.R
#	R/bin.R
#	R/coord-.R
#	R/coord-cartesian-.R
#	R/facet-.R
#	R/fortify.R
#	R/geom-.R
#	R/geom-defaults.R
#	R/geom-label.R
#	R/ggproto.R
#	R/guide-.R
#	R/guide-axis.R
#	R/guides-.R
#	R/layer.R
#	R/margins.R
#	R/plot-build.R
#	R/plot-construction.R
#	R/plot.R
#	R/position-.R
#	R/save.R
#	R/scale-.R
#	R/scale-colour.R
#	R/stat-.R
#	R/stat-ellipse.R
#	R/theme-current.R
#	R/theme-elements.R
#	R/theme.R
#	README.Rmd
#	cran-comments.md
#	man/coord_polar.Rd
#	man/element.Rd
#	man/get_geom_defaults.Rd
#	man/is_tests.Rd
#	man/stat_connect.Rd
#	man/stat_ellipse.Rd
#	revdep/README.md
#	revdep/cran.md
#	revdep/failures.md
#	revdep/problems.md
#	tests/testthat/_snaps/utilities-checks.md
#	tests/testthat/test-utilities-checks.R
Merge branch 'main' into S7_elements

# Conflicts:
#	R/geom-label.R
#	R/guide-.R
#	R/guide-axis.R
#	R/margins.R
#	R/theme-elements.R
#	R/theme.R
#	man/element.Rd
#	man/is_tests.Rd
#	tests/testthat/test-theme.R
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