Skip to content

Commit

Permalink
simplify animint() logic and fix bug for two un-named ggplots
Browse files Browse the repository at this point in the history
  • Loading branch information
tdhock committed Jun 7, 2024
1 parent c7b40c2 commit 9ba62a2
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 23 deletions.
26 changes: 7 additions & 19 deletions R/z_print.R
Original file line number Diff line number Diff line change
Expand Up @@ -82,34 +82,22 @@ print.animint <- function(x, ...){
##' scale_size_animint(pixel.range=c(2,20), breaks=10^(4:9)))
animint <- function(...){
L <- list(...)

# Check if argument list is empty
if(length(L) == 0) {
stop("No arguments passed to animint. Arguments should include ggplots(1 or more) and options(0 or more)")
}

# Generate a list of duplicate named arguments
duplicate_args <- names(L)[duplicated(names(L))]

if (length(duplicate_args) > 0) {
stop(paste("Duplicate arguments are passed to animint. Duplicate arguments found:",
paste(duplicate_args, collapse = ", ")))
}
default.name.vec <- plot.num.vec <- paste0("plot", seq_along(L))
match.name.list <- lapply(match.call()[-1], paste)
first.name.vec <- sapply(match.name.list, "[", 1)
sub.name.vec <- gsub("[^a-zA-Z0-9]", "", first.name.vec)
name.ok <- grepl("^[a-zA-Z][a-zA-Z0-9]*$", sub.name.vec)
use.name <- sapply(match.name.list, length)==1 & name.ok
default.name.vec[use.name] <- sub.name.vec[use.name]
plot.num.vec <- paste0("plot", seq(1, length(L)*2))
default.name.vec <- plot.num.vec[!plot.num.vec %in% names(L)][1:length(L)]
if(is.null(names(L))){
names(L) <- default.name.vec
}
still.empty <- names(L)==""
still.empty <- is.na(names(L)) | names(L)==""
names(L)[still.empty] <- default.name.vec[still.empty]
name.tab <- table(names(L))
is.rep <- names(L) %in% names(name.tab)[1 < name.tab]
names(L)[is.rep] <- plot.num.vec[is.rep]
rep.names <- names(name.tab)[1 < name.tab]
if(length(rep.names)){
stop("Duplicate named arguments are passed to animint. Duplicate argument names found: ", paste(rep.names, collapse=","))
}
structure(L, class="animint")
}

20 changes: 16 additions & 4 deletions tests/testthat/test-compiler-errors.R
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,20 @@ test_that("warning for _off param without clickSelects", {

test_that("animint(out.dir = 'dir1', out.dir = 'dir2') is an error", {
expect_error({
animint(out.dir = 'dir1', out.dir = 'dir2')
}, "Duplicate arguments are passed to animint. Duplicate arguments found: out.dir")
viz <- animint(out.dir = 'dir1', out.dir = 'dir2')
}, "Duplicate named arguments are passed to animint. Duplicate argument names found: out.dir")
})

test_that("animint(plot1, plot2) is ok", {
viz <- animint(ggplot(), ggplot())
(computed.names <- names(viz))
expect_identical(computed.names, c("plot1","plot2"))
})

test_that("animint(ggplot(), ggplot(), plot1=ggplot()) is ok", {
viz <- animint(ggplot(), ggplot(), plot1=ggplot())
(computed.names <- names(viz))
expect_identical(computed.names, c("plot2","plot3","plot1"))
})

test_that("animint() is an error", {
Expand All @@ -208,7 +220,7 @@ test_that("Same argument passed to aes and geom is an error", {
plot = scatter
)
expect_error({
animint2dir(viz, open.browser=FALSE)
animint2dir(viz, open.browser=FALSE)
}, "Same visual property cannot be defined in both aes and geom. Property defined in aes:alpha. Property defined in geom:alpha. The visual property needs only be defined in one place, so if it should be different for each rendered geom, but not depend on selection state, then it should be defined in aes; but if the property should depend on the selection state then it should be defined in geom")
})

Expand Down Expand Up @@ -240,4 +252,4 @@ test_that("alpha_off without clickSelects is a warning", {
expect_warning({
animint2dir(viz, open.browser=FALSE)
}, "geom1_path_plot has alpha_off which is not used because this geom has no clickSelects; please specify clickSelects or remove alpha_off")
})
})

0 comments on commit 9ba62a2

Please sign in to comment.