Skip to content

Polishes triggered by revdep failures #6506

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 24 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
c4ffa57
don't extract <element_blank>@hjust
teunbrand Jun 13, 2025
6c13d47
ensure `if` switch is scalar
teunbrand Jun 13, 2025
16c3084
contingency where `merge_element(..., e2 = <S3_element>)`
teunbrand Jun 16, 2025
bad1e32
cast a wider net for function matching
teunbrand Jun 16, 2025
593f326
Fix NOTE: no visible binding for global variable 'properties'
teunbrand Jun 16, 2025
e169794
stabilise missingness detection for 2D structures
teunbrand Jun 16, 2025
7a886b8
avoid recursion issue when `theme` is missing
teunbrand Jun 17, 2025
a15f1ac
avoid renaming argument for S3 method consistency purposes
teunbrand Jun 17, 2025
6743479
demote warning to message
teunbrand Jun 18, 2025
ec193c0
stop complaints about 0-length labels, which is commonly used in labe…
teunbrand Jun 18, 2025
2429d31
allow merging <S3> + <S3>
teunbrand Jun 18, 2025
27465ff
fix #6521
teunbrand Jun 19, 2025
5cb124c
fix #6513
teunbrand Jun 19, 2025
afb019e
avoid rare error when using GeomSf based extensions
teunbrand Jun 19, 2025
b82fc4a
`label.size` should be soft-deprecated first
teunbrand Jun 20, 2025
29e08d3
Merge branch 'main' into revdep_deprecated_theme
teunbrand Jun 23, 2025
e7e08d5
put `convert` into `local()` too
teunbrand Jun 23, 2025
3655a59
restore accent colour for `geom_contour()`
teunbrand Jun 23, 2025
3ab6b3a
new revdepcheck
thomasp85 Jun 23, 2025
fd63768
Merge branch 'revdep_deprecated_theme' of https://github.com/teunbran…
teunbrand Jun 23, 2025
2b22612
don't prepend `""` as class
teunbrand Jun 24, 2025
db292e6
mark the binning change as a breaking change
teunbrand Jun 26, 2025
78e6aec
fix #6531
teunbrand Jun 26, 2025
d3d2956
fix #6529
teunbrand Jun 26, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion R/coord-radial.R
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
}
if (lifecycle::is_present(direction)) {
deprecate_warn0(
"3.5.2", "coord_radial(direction)", "coord_radial(reverse)"
"4.0.0", "coord_radial(direction)", "coord_radial(reverse)"

Check warning on line 139 in R/coord-radial.R

View check run for this annotation

Codecov / codecov/patch

R/coord-radial.R#L139

Added line #L139 was not covered by tests
)
reverse <- switch(reverse, "r" = "thetar", "theta")
}
Expand Down
2 changes: 1 addition & 1 deletion R/coord-transform.R
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
#' @export
coord_trans <- function(...) {
deprecate_soft0(
"3.5.2",
"4.0.0",

Check warning on line 116 in R/coord-transform.R

View check run for this annotation

Codecov / codecov/patch

R/coord-transform.R#L116

Added line #L116 was not covered by tests
"coord_trans()",
"coord_transform()"
)
Expand Down
2 changes: 1 addition & 1 deletion R/facet-wrap.R
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ wrap_layout <- function(id, dims, dir) {
# Should only occur when `as.table` was not incorporated into `dir`
dir <- switch(dir, h = "lt", v = "tl")
deprecate_soft0(
"3.5.2",
"4.0.0",
what = I("Internal use of `dir = \"h\"` and `dir = \"v\"` in `facet_wrap()`"),
details = I(c(
"The `dir` argument should incorporate the `as.table` argument.",
Expand Down
2 changes: 1 addition & 1 deletion R/geom-boxplot.R
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ GeomBoxplot <- ggproto("GeomBoxplot", Geom,
setup_params = function(data, params) {
if ("fatten" %in% names(params)) {
deprecate_soft0(
"3.6.0", "geom_boxplot(fatten)",
"4.0.0", "geom_boxplot(fatten)",
"geom_boxplot(median.linewidth)"
)
} else {
Expand Down
2 changes: 1 addition & 1 deletion R/geom-crossbar.R
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ GeomCrossbar <- ggproto("GeomCrossbar", Geom,
setup_params = function(data, params) {
if (lifecycle::is_present(params$fatten %||% deprecated())) {
deprecate_soft0(
"3.6.0", "geom_crossbar(fatten)",
"4.0.0", "geom_crossbar(fatten)",
"geom_crossbar(middle.linewidth)"
)
} else {
Expand Down
4 changes: 2 additions & 2 deletions R/geom-errorbar.R
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ GeomErrorbarh <- ggproto(
"GeomErrorbarh", GeomErrorbar,
setup_params = function(data, params) {
deprecate_soft0(
"3.5.2", "geom_errobarh()", "geom_errorbar(orientation = \"y\")",
"4.0.0", "geom_errobarh()", "geom_errorbar(orientation = \"y\")",
id = "no-more-errorbarh"
)
GeomLinerange$setup_params(data, params)
Expand All @@ -87,7 +87,7 @@ geom_errorbar <- make_constructor(GeomErrorbar, orientation = NA)
#' `geom_errorbar(orientation = "y")` instead.
geom_errorbarh <- function(..., orientation = "y") {
deprecate_soft0(
"3.5.2", "geom_errobarh()", "geom_errorbar(orientation = \"y\")",
"4.0.0", "geom_errobarh()", "geom_errorbar(orientation = \"y\")",
id = "no-more-errorbarh"
)
geom_errorbar(..., orientation = orientation)
Expand Down
2 changes: 1 addition & 1 deletion R/geom-label.R
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

extra_args <- list2(...)
if (lifecycle::is_present(label.size)) {
deprecate_warn0("3.5.0", "geom_label(label.size)", "geom_label(linewidth)")
deprecate_soft0("3.5.0", "geom_label(label.size)", "geom_label(linewidth)")

Check warning on line 31 in R/geom-label.R

View check run for this annotation

Codecov / codecov/patch

R/geom-label.R#L31

Added line #L31 was not covered by tests
extra_args$linewidth <- extra_args$linewidth %||% label.size
}

Expand Down
2 changes: 1 addition & 1 deletion R/geom-pointrange.R
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ GeomPointrange <- ggproto("GeomPointrange", Geom,

setup_params = function(data, params) {
if (lifecycle::is_present(params$fatten %||% deprecated())) {
deprecate_soft0("3.6.0", "geom_pointrange(fatten)", I("the `size` aesthetic"))
deprecate_soft0("4.0.0", "geom_pointrange(fatten)", I("the `size` aesthetic"))
} else {
# For backward compatibility reasons
params$fatten <- 4
Expand Down
2 changes: 1 addition & 1 deletion R/geom-violin.R
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
extra <- list()
if (lifecycle::is_present(draw_quantiles)) {
deprecate_soft0(
"3.6.0",
"4.0.0",

Check warning on line 114 in R/geom-violin.R

View check run for this annotation

Codecov / codecov/patch

R/geom-violin.R#L114

Added line #L114 was not covered by tests
what = "geom_violin(draw_quantiles)",
with = "geom_violin(quantiles.linetype)"
)
Expand Down
2 changes: 1 addition & 1 deletion R/guide-colorsteps.R
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ GuideColoursteps <- ggproto(
params$title <- scale$make_title(params$title, scale$name, title)

limits <- c(params$decor$min[1], params$decor$max[nrow(params$decor)])
if (params$reverse) {
if (isTRUE(params$reverse)) {
limits <- rev(limits)
}
params$key$.value <- rescale(params$key$.value, from = limits)
Expand Down
3 changes: 2 additions & 1 deletion R/guides-.R
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ Guides <- ggproto(
#
# The resulting guide is then drawn in ggplot_gtable

build = function(self, scales, layers, labels, layer_data, theme = theme()) {
build = function(self, scales, layers, labels, layer_data, theme = NULL) {

# Empty guides list
custom <- self$get_custom()
Expand All @@ -300,6 +300,7 @@ Guides <- ggproto(
}

# Merge and process layers
theme <- theme %||% theme()
guides$merge()
guides$process_layers(layers, layer_data, theme)
if (length(guides$guides) == 0) {
Expand Down
5 changes: 3 additions & 2 deletions R/labels.R
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ setup_plot_labels <- function(plot, layers, data) {
# `geom_function()`. We can display these labels anyway, so we include them.
plot_labels <- plot@labels
known_labels <- c(names(labels), fn_fmls_names(labs), "x", "y")
extra_labels <- setdiff(names(plot_labels), known_labels)
extra_labels <- names(plot_labels)[lengths(plot_labels) > 0]
extra_labels <- setdiff(extra_labels, known_labels)

if (length(extra_labels) > 0) {

Expand All @@ -92,7 +93,7 @@ setup_plot_labels <- function(plot, layers, data) {
extra_labels <- paste0("{.field ", extra_labels, "} : ", warn_labels)
names(extra_labels) <- rep("*", length(extra_labels))

cli::cli_warn(c(
cli::cli_inform(c(
"Ignoring unknown labels:",
extra_labels
))
Expand Down
7 changes: 6 additions & 1 deletion R/layer-sf.R
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,12 @@
geom_column <- function(data) {
w <- which(vapply(data, inherits, TRUE, what = "sfc"))
if (length(w) == 0) {
"geometry" # avoids breaks when objects without geometry list-column are examined
if (!is.character(data[["geometry"]])) {
"geometry" # avoids breaks when objects without geometry list-column are examined
} else {
# Avoids a rare case where computed_geom_params$legend is present but there is no actual geometry column
""

Check warning on line 85 in R/layer-sf.R

View check run for this annotation

Codecov / codecov/patch

R/layer-sf.R#L85

Added line #L85 was not covered by tests
}
} else {
# this may not be best in case more than one geometry list-column is present:
if (length(w) > 1)
Expand Down
6 changes: 3 additions & 3 deletions R/plot-build.R
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Collaborator Author

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.

UseMethod("ggplot_gtable")
}

#' @export
ggplot_gtable.default <- function(plot) {
gtable_ggplot(plot)
ggplot_gtable.default <- function(data) {
gtable_ggplot(data)
}

#' Generate a ggplot2 plot grob.
Expand Down
1 change: 1 addition & 0 deletions R/plot.R
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ S7::method(convert, list(from = class_ggplot, to = S7::class_list)) <-
function(from, to) {
S7::props(from)
}
rm(`convert`)

S7::method(as.list, class_ggplot) <- function(x, ...) convert(x, S7::class_list)
rm(`as.list`)
2 changes: 1 addition & 1 deletion R/position-dodge.R
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ PositionDodge <- ggproto("PositionDodge", Position,
data$order <- xtfrm( # xtfrm makes anything 'sortable'
data$order %||% ave(data$group, data$x, data$PANEL, FUN = match_sorted)
)
if (params$reverse) {
if (isTRUE(params$reverse)) {
data$order <- -data$order
}
if (is.null(params$n)) { # preserve = "total"
Expand Down
2 changes: 1 addition & 1 deletion R/position-stack.R
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ PositionStack <- ggproto("PositionStack", Position,
flipped_aes <- has_flipped_aes(data)
data <- flip_data(data, flipped_aes)
var <- self$var %||% stack_var(data)
if (!vec_duplicate_any(data$x)) {
if (!vec_duplicate_any(data$x) && !isTRUE(self$fill)) {
# We skip stacking when all data have different x positions so that
# there is nothing to stack
var <- NULL
Expand Down
2 changes: 1 addition & 1 deletion R/scale-.R
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,7 @@ ScaleDiscrete <- ggproto("ScaleDiscrete", Scale,
vec_slice(pal, match(as.character(x), limits, nomatch = vec_size(pal)))

if (!is.na(na_value)) {
vec_slice(pal_match, is.na(x)) <- na_value
vec_slice(pal_match, vec_detect_missing(x)) <- na_value
}
pal_match
},
Expand Down
2 changes: 1 addition & 1 deletion R/stat-summary-2d.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ StatSummary2d <- ggproto(
params$drop <- !identical(params$drop, "none")
}

params <- fix_bin_params(params, fun = snake_class(self), version = "3.5.2")
params <- fix_bin_params(params, fun = snake_class(self), version = "4.0.0")
vars <- c("origin", "binwidth", "breaks", "center", "boundary")
params[vars] <- lapply(params[vars], dual_param, default = NULL)
params$closed <- dual_param(params$closed, list(x = "right", y = "right"))
Expand Down
7 changes: 7 additions & 0 deletions R/stat-summary-bin.R
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ make_summary_fun <- function(fun.data, fun, fun.max, fun.min, fun.args) {
force(fun.min)
force(fun.args)

as_function <- function(x) {
if (is.character(x)) {
x <- match.fun(x)
}
rlang::as_function(x)
}

if (!is.null(fun.data)) {
# Function that takes complete data frame as input
fun.data <- as_function(fun.data)
Expand Down
2 changes: 1 addition & 1 deletion R/stat-ydensity.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ StatYdensity <- ggproto(

if (!is.null(params$draw_quantiles)) {
deprecate_soft0(
"3.6.0",
"4.0.0",
what = "stat_ydensity(draw_quantiles)",
with = "stat_ydensity(quantiles)"
)
Expand Down
24 changes: 13 additions & 11 deletions R/theme.R
Original file line number Diff line number Diff line change
Expand Up @@ -518,9 +518,9 @@
)
if (is.null(elements[["legend.title"]])) {
elements$legend.title <- element_text(hjust = elements$legend.title.align)
} else {
elements$legend.title$hjust <- elements$legend.title$hjust %||%
elements$legend.title.align
} else if (!is_theme_element(elements$legend.title, "blank")) {
elements$legend.title$hjust <-
try_prop(elements$legend.title, "hjust", elements$legend.title.align)

Check warning on line 523 in R/theme.R

View check run for this annotation

Codecov / codecov/patch

R/theme.R#L521-L523

Added lines #L521 - L523 were not covered by tests
}
elements$legend.title.align <- NULL
}
Expand All @@ -531,9 +531,9 @@
)
if (is.null(elements[["legend.text"]])) {
elements$legend.text <- element_text(hjust = elements$legend.text.align)
} else {
elements$legend.text$hjust <- elements$legend.text$hjust %||%
elements$legend.text.align
} else if (!is_theme_element(elements$legend.text, "blank")) {
elements$legend.text$hjust <-
try_prop(elements$legend.text, "hjust", elements$legend.text.align)

Check warning on line 536 in R/theme.R

View check run for this annotation

Codecov / codecov/patch

R/theme.R#L534-L536

Added lines #L534 - L536 were not covered by tests
}
elements$legend.text.align <- NULL
}
Expand Down Expand Up @@ -980,7 +980,7 @@
# Backward compatbility
# TODO: deprecate next release cycle
is_old_element <- !S7::S7_inherits(e1) && inherits(e1, "element")
if (is_old_element && is_theme_element(e2)) {
if (is_old_element && (is_theme_element(e2) || inherits(e2, "element"))) {
return(combine_s3_elements(e1, e2))
}

Expand All @@ -999,22 +999,24 @@
}
}

parent_props <- if (S7::S7_inherits(e2)) S7::props(e2) else unclass(e2)

Comment on lines +1002 to +1003
Copy link
Collaborator Author

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

# If e1 has any NULL properties, inherit them from e2
n <- S7::prop_names(e1)[lengths(S7::props(e1)) == 0]
S7::props(e1)[n] <- S7::props(e2)[n]
S7::props(e1)[n] <- parent_props[n]

# Calculate relative sizes
if (is_rel(try_prop(e1, "size"))) {
e1@size <- e2@size * unclass(e1@size)
e1@size <- parent_props$size * unclass(e1@size)
}

# Calculate relative linewidth
if (is_rel(try_prop(e1, "linewidth"))) {
e1@linewidth <- e2@linewidth * unclass(e1@linewidth)
e1@linewidth <- parent_props$linewidth * unclass(e1@linewidth)
}

if (is_theme_element(e1, "text")) {
e1@margin <- combine_elements(e1@margin, e2@margin)
e1@margin <- combine_elements(e1@margin, parent_props$margin)
}

# If e2 is 'richer' than e1, fill e2 with e1 parameters
Expand Down
2 changes: 1 addition & 1 deletion man/gtable_ggplot.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 7 additions & 4 deletions tests/testthat/_snaps/labels.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@

# warnings are thrown for unknown labels

Ignoring unknown labels:
* foo : "i don't exist"
* bar : <function>
* qux : "expression(me * neither)"
Code
p <- ggplot_build(p)
Message
Ignoring unknown labels:
* foo : "i don't exist"
* bar : <function>
* qux : "expression(me * neither)"

# plot.tag.position rejects invalid input

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-labels.R
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ test_that("warnings are thrown for unknown labels", {
bar = function(x) "i don't exist either",
qux = expression(me * neither)
)
expect_snapshot_warning(ggplot_build(p))
expect_snapshot(p <- ggplot_build(p))
})

test_that("plot.tag.position rejects invalid input", {
Expand Down
6 changes: 6 additions & 0 deletions tests/testthat/test-position-stack.R
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ test_that("position_stack() can stack correctly when ymax is NA", {
expect_equal(get_layer_data(p)$y, c(1, 2))
})

test_that("position_fill() handles one group per position case", {
df <- data_frame0(x = c("A", "B", "C"), y = 1:3)
p <- ggplot(df, aes(x, y)) + geom_col(position = "fill")
expect_equal(get_layer_data(p)$y, c(1, 1, 1))
})

# Visual tests ------------------------------------------------------------

test_that("Stacking produces the expected output", {
Expand Down