-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Adopt S7 in most places #6364
base: main
Are you sure you want to change the base?
Adopt S7 in most places #6364
Conversation
|
||
#' @export | ||
print.uneval <- function(x, ...) { | ||
`print.ggplot2::mapping` <- function(x, ...) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we have an S3 generic we need for both S3 and S7 objects, we need to implement methods for the S7 objects in the S3 fashion, because using S7::method()
will invalidate the S3 methods.
Older versions of R seem to get stuck on dispatch with |
class_gg <- S7::new_class("gg", abstract = TRUE) | ||
class_S3_gg <- S7::new_S3_class("gg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add note to remove at some point
TODO: Remove class gymnastics |
This PR aims to fix a large part of #6352.
Briefly it implements the following S7 classes:
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.