Skip to content

Reimplement classes in S7 #6364

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 69 commits into
base: main
Choose a base branch
from
Open

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Mar 17, 2025

This PR aims to fix a large part of #6352.

Briefly it implements the following S7 classes:

  • class_ggplot
  • class_ggplot_built
  • class_mapping
  • class_labels
  • class_theme

I've put in the usual extractors and replacers ([, [[, $ and their <- methods) for the ggplot/ggplot_built classes for backwards compatibility. The code internally now uses @ though, so that we can deprecate the usual extractors/replacers later.

@thomasp85 thomasp85 self-requested a review April 22, 2025 09:13
@teunbrand teunbrand added this to the ggplot2 4.0.0 milestone May 6, 2025
teunbrand added 6 commits May 14, 2025 09:14
Merge branch 'main' into S7_elements

# Conflicts:
#	DESCRIPTION
Merge branch 'main' into S7_objects

# Conflicts:
#	DESCRIPTION

#' @export
print.uneval <- function(x, ...) {
`print.ggplot2::mapping` <- function(x, ...) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know the timeline for resolving this?

teunbrand and others added 4 commits May 14, 2025 11:00
Merge branch 'S7_objects' into S7_elements

# Conflicts:
#	NAMESPACE
#	R/plot-construction.R
#	R/theme.R
#	R/zzz.R
#	man/ggplot_add.Rd
#	tests/testthat/test-theme.R
@teunbrand teunbrand changed the title Adopt S7 in most places Reimplement classes in S7 May 14, 2025
@teunbrand
Copy link
Collaborator Author

#6355 is now merged in via teunbrand#16

@teunbrand teunbrand mentioned this pull request May 14, 2025
@teunbrand
Copy link
Collaborator Author

I'm slightly annoyed that we have a class_-prefix for most classes now, e.g. class_ggplot, but not for the theme elements which are still element_line etc.

@thomasp85
Copy link
Member

I'm open to suggestions, but can also live a long and happy life with the discrepancy

@teunbrand
Copy link
Collaborator Author

Alright let's not worry about it too much then :)

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