Skip to content
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

Check ellipses in several places #6031

Merged
merged 7 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@
* `theme_classic()` now has black ticks and text instead of dark gray. In
addition, `theme_classic()`'s axis line end is `"square"` (@teunbrand, #5978).
* {tibble} is now suggested instead of imported (@teunbrand, #5986)
* The ellipsis argument is now checked in `fortify()`, `get_alt_text()`,
`labs()` and several guides (@teunbrand, #3196).

# ggplot2 3.5.1

Expand Down
7 changes: 5 additions & 2 deletions R/fortify.R
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
#' @seealso [fortify.lm()]
#' @param model model or other R object to convert to data frame
#' @param data original dataset, if needed
#' @param ... other arguments passed to methods
#' @inheritParams rlang::args_dots_used
#' @export
fortify <- function(model, data, ...) UseMethod("fortify")
fortify <- function(model, data, ...) {
warn_dots_used()
UseMethod("fortify")
}

#' @export
fortify.data.frame <- function(model, data, ...) model
Expand Down
1 change: 1 addition & 0 deletions R/guide-legend.R
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,7 @@ deprecated_guide_args <- function(
default.unit = "line",
...,
.call = caller_call()) {
warn_dots_used(call = .call)

args <- names(formals(deprecated_guide_args))
args <- setdiff(args, c("theme", "default.unit", "...", ".call"))
Expand Down
28 changes: 24 additions & 4 deletions R/labels.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ update_labels <- function(p, labels) {

# Called in `ggplot_build()` to set default labels not specified by user.
setup_plot_labels <- function(plot, layers, data) {
# Initiate from user-defined labels
labels <- plot$labels
# Initiate empty labels
labels <- list()

# Find labels from every layer
for (i in seq_along(layers)) {
Expand Down Expand Up @@ -65,7 +65,26 @@ setup_plot_labels <- function(plot, layers, data) {
labels <- defaults(labels, current)
}
}
labels

# Warn for spurious labels that don't have a mapping.
# Note: sometimes, 'x' and 'y' might not have a mapping, like in
# `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)

if (length(extra_labels) > 0) {
extra_labels <- paste0(
"{.code ", extra_labels, " = \"", plot_labels[extra_labels], "\"}"
)
names(extra_labels) <- rep("*", length(extra_labels))
cli::cli_warn(c(
"Ignoring unknown labels:",
extra_labels
))
}

defaults(plot_labels, labels)
}

#' Modify axis, legend, and plot labels
Expand Down Expand Up @@ -168,7 +187,7 @@ ggtitle <- function(label, subtitle = waiver()) {
#' text from the information stored in the plot.
#'
#' @param p a ggplot object
#' @param ... Currently ignored
#' @inheritParams rlang::args_dots_used
#'
#' @return A text string
#'
Expand All @@ -191,6 +210,7 @@ ggtitle <- function(label, subtitle = waiver()) {
#' get_alt_text(p)
#'
get_alt_text <- function(p, ...) {
warn_dots_used()
UseMethod("get_alt_text")
}
#' @export
Expand Down
12 changes: 12 additions & 0 deletions R/utilities.R
Original file line number Diff line number Diff line change
Expand Up @@ -843,6 +843,18 @@ as_unordered_factor <- function(x) {
x
}

warn_dots_used <- function(env = caller_env(), call = caller_env()) {
check_dots_used(
env = env, call = call,
# Demote from error to warning
error = function(cnd) {
# cli uses \f as newlines, not \n
msg <- gsub("\n", "\f", cnd_message(cnd))
cli::cli_warn(msg, call = call)
}
)
}

# Shim for scales/#424
col_mix <- function(a, b, amount = 0.5) {
input <- vec_recycle_common(a = a, b = b, amount = amount)
Expand Down
2 changes: 1 addition & 1 deletion man/fortify.Rd

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

2 changes: 1 addition & 1 deletion man/get_alt_text.Rd

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

11 changes: 11 additions & 0 deletions tests/testthat/_snaps/guides.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,14 @@
# dots are checked when making guides

Ignoring unknown argument to `guide_axis()`: `foo`.

---

Arguments in `...` must be used.
x Problematic argument:
* foo = "bar"
i Did you misspell an argument name?

# Using non-position guides for position scales results in an informative error

`guide_legend()` cannot be used for x, xmin, xmax, or xend.
Expand Down
12 changes: 12 additions & 0 deletions tests/testthat/_snaps/labels.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,18 @@
Output
[1] "A plot showing class on a discrete x-axis and count on a continuous y-axis using a bar layer."

# get_alt_text checks dots

Arguments in `...` must be used.
x Problematic argument:
* foo = "bar"
i Did you misspell an argument name?

# warnings are thrown for unknown labels

Ignoring unknown labels:
* `foo = "bar"`

# plot.tag.position rejects invalid input

The `plot.tag.position` theme element must be a <character/numeric/integer> object.
Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/_snaps/plot.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@
`data` cannot be a function.
i Have you misspelled the `data` argument in `ggplot()`

---

Arguments in `...` must be used.
x Problematic argument:
* foobar = "nonsense"
i Did you misspell an argument name?

# construction have user friendly errors

Cannot use `+` with a single argument.
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-guides.R
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ test_that("show.legend handles named vectors", {
expect_equal(n_legends(p), 1)
})

test_that("dots are checked when making guides", {
expect_snapshot_warning(
new_guide(foo = "bar", super = GuideAxis)
)
expect_snapshot_warning(
guide_legend(foo = "bar")
)
})

test_that("axis_label_overlap_priority always returns the correct number of elements", {
expect_identical(axis_label_priority(0), numeric(0))
expect_setequal(axis_label_priority(1), seq_len(1))
Expand Down
9 changes: 9 additions & 0 deletions tests/testthat/test-labels.R
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,15 @@ test_that("alt text can take a function", {
expect_snapshot(get_alt_text(p))
})

test_that("get_alt_text checks dots", {
expect_snapshot_warning(get_alt_text(ggplot(), foo = "bar"))
})

test_that("warnings are thrown for unknown labels", {
p <- ggplot(mtcars, aes(mpg, disp)) + geom_point() + labs(foo = 'bar')
expect_snapshot_warning(ggplot_build(p))
})

test_that("plot.tag.position rejects invalid input", {
p <- ggplot(mtcars, aes(mpg, disp)) + geom_point() + labs(tag = "Fig. A)")

Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-plot.R
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
test_that("ggplot() throws informative errors", {
expect_snapshot_error(ggplot(mapping = letters))
expect_snapshot_error(ggplot(data))
expect_snapshot_warning(ggplot(foobar = "nonsense"))
})

test_that("construction have user friendly errors", {
Expand Down
Loading