From 594ef12239722e741ed5bf226ec26d5e7a26a27c Mon Sep 17 00:00:00 2001
From: Toby Hocking <toby.hocking@nau.edu>
Date: Fri, 7 Jun 2024 07:41:05 -0400
Subject: [PATCH] simplify animint() logic and fix bug for two un-named ggplots

---
 R/z_print.R                           | 26 +++++++-------------------
 tests/testthat/test-compiler-errors.R | 20 ++++++++++++++++----
 2 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/R/z_print.R b/R/z_print.R
index c739e4d8d..121533a0e 100644
--- a/R/z_print.R
+++ b/R/z_print.R
@@ -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")
 }
 
diff --git a/tests/testthat/test-compiler-errors.R b/tests/testthat/test-compiler-errors.R
index f497b93a8..8d9df6da2 100644
--- a/tests/testthat/test-compiler-errors.R
+++ b/tests/testthat/test-compiler-errors.R
@@ -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", {
@@ -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")
 })
 
@@ -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")
-})
\ No newline at end of file
+})