-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Polishes triggered by revdep failures #6506
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
base: main
Are you sure you want to change the base?
Conversation
Also snuck in another tiny issue I ran into |
When we use |
@@ -317,13 +317,13 @@ S7::method(gtable_ggplot, class_ggplot_built) <- function(data) { | |||
# TODO: the S3 generic should be phased out once S7 is adopted more widely | |||
#' @rdname gtable_ggplot | |||
#' @export | |||
ggplot_gtable <- function(plot) { | |||
ggplot_gtable <- function(data) { |
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.
The argument was data
before, which I changed to plot
because it felt more descriptive. However, this got us into trouble with S3 method/generic consistency.
parent_props <- if (S7::S7_inherits(e2)) S7::props(e2) else unclass(e2) | ||
|
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.
This is so that parent_props$name
works, regardless of S3/S7
…d/ggplot2 into revdep_deprecated_theme
INITIALLY: Fix revdep failure: don't extract <element_blank>@hjust
This affects only 2 packages (smallsets and utile.visuals), but it is our faux pas.
Essentially, because
element_blank
does not have thehjust
property, trying to extract it will error.In this PR, we are more careful in trying to extract this property.