Throw error when same argument is passed to aes and geom

DESCRIPTION

@@ -1,6 +1,6 @@
 Package: animint2
 Title: Animated Interactive Grammar of Graphics
-Version: 2024.2.4
+Version: 2024.3.12
 URL: https://animint.github.io/animint2/
 BugReports: https://github.com/animint/animint2/issues
 Authors@R: c(

NEWS.md

@@ -1,3 +1,7 @@
+# Changes in version 2024.3.12 (PR#119)
+
+  - Add validation checks for duplicate args passed geom and aes
+
 # Changes in version 2024.2.4 (PR#116)
 
   - Add validation checks for duplicate and missing args passed to animint.

R/geom-.r

@@ -147,7 +147,7 @@ Geom <- gganimintproto("Geom",
   pre_process = function(g, g.data, ranges){
     list(g = g, g.data = g.data)
   },
-
+  
   ## Save a layer to disk, save and return meta-data.
   ## l- one layer of the ggplot object.
   ## d- one layer of calculated data from ggplot_build(p).
@@ -174,6 +174,19 @@ Geom <- gganimintproto("Geom",
     ## e.g. colour.
     ## 'colour', 'size' etc. have been moved to aes_params
     g$params <- getLayerParams(l)
+    off_params <- sub("_off", "", grep("_off", names(l$extra_params), value = TRUE))
+    geom_params <- c(names(l$aes_params), off_params)
+    duplicate_params <- intersect(geom_params, names(g$aes))
+
+
+    if(length(duplicate_params) > 0) {
+      duplicate_on <- intersect(names(l$aes_params), duplicate_params)
+      duplicate_off <- setdiff(paste0(intersect(off_params, duplicate_params),"_off"), "_off")
+      duplicate_geom <- c(duplicate_on, duplicate_off)
+      stop(paste("Same argument cannot be passed to both aes and geom. Argument passed to aes:", 
+                 paste(duplicate_params, collapse = ", "), ". Arguments passed to geom:", paste(duplicate_geom, collapse = ", "),
+                 ". 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"))
+    }
 
     ## Make a list of variables to use for subsetting. subset_order is the
     ## order in which these variables will be accessed in the recursive

tests/testthat/test-compiler-empty-data.r

@@ -56,29 +56,6 @@ test_that("layers with empty data are silently omitted with facet_grid", {
   expect_equal(nrow(layer_data(d, 2)), nrow(mtcars))
 })
 
-
-test_that("empty data overrides plot defaults", {
-  # Should error when totally empty data frame because there's no x and y
-  d <- ggplot(mtcars, aes(mpg, wt)) +
-    geom_point() +
-    geom_point(data = data.frame())
-  expect_error(layer_data(d), "not found")
-
-  # No extra points when x and y vars don't exist but are set
-  d <- ggplot(mtcars, aes(mpg, wt)) +
-    geom_point() +
-    geom_point(data = data.frame(), x = 20, y = 3)
-  expect_equal(nrow(layer_data(d, 1)), nrow(mtcars))
-  expect_equal(nrow(layer_data(d, 2)), 0)
-
-  # No extra points when x and y vars are empty, even when aesthetics are set
-  d <- ggplot(mtcars, aes(mpg, wt)) +
-    geom_point() +
-    geom_point(data = df0, x = 20, y = 3)
-  expect_equal(nrow(layer_data(d, 1)), nrow(mtcars))
-  expect_equal(nrow(layer_data(d, 2)), 0)
-})
-
 test_that("layer inherits data from plot when data = NULL", {
   d <- ggplot(mtcars, aes(mpg, wt)) +
     geom_point(data = NULL)

tests/testthat/test-compiler-errors.R

@@ -175,9 +175,8 @@ test_that("no warning for position=stack without showSelected", {
 test_that("warning for _off param without clickSelects", {
   viz.point1 <- list(
     pointone = ggplot()+
-    geom_point(aes(x = wt, y = mpg, colour = disp),
+    geom_point(aes(x = wt, y = mpg),
     size = 10,
-    colour="red",
     alpha_off=0.6,
     colour_off="transparent",
     data = mtcars))
@@ -197,3 +196,48 @@ test_that("animint() is an error", {
     animint()
   }, "No arguments passed to animint. Arguments should include ggplots(1 or more) and options(0 or more)", fixed=TRUE)
 })
+
+test_that("Same argument passed to aes and geom is an error", {
+  scatter <- ggplot()+geom_path(aes(
+    x=life.expectancy, 
+    y=fertility.rate, 
+    color=region,
+    group=country, alpha=population),alpha=0.5,
+    data=WorldBank)
+  viz <- list(
+    plot = scatter
+  )
+  expect_error({
+      animint2dir(viz, open.browser=FALSE)
+  }, "Same argument cannot be passed to both aes and geom. Argument passed to aes: alpha . Arguments passed to 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")
+})
+
+test_that("alpha and alpha_off passed to aes and geom is an error", {
+  scatter <- ggplot()+geom_path(aes(
+    x=life.expectancy, 
+    y=fertility.rate, 
+    color=region,
+    group=country, alpha=population),clickSelects="country", alpha_off=0.5,
+    data=WorldBank)
+  viz <- list(
+    plot = scatter
+  )
+  expect_error({
+    animint2dir(viz, open.browser=FALSE)
+  }, "Same argument cannot be passed to both aes and geom. Argument passed to aes: alpha . Arguments passed to geom: alpha_off . 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")
+})
+
+test_that("alpha_off without clickSelects is a warning", {
+  scatter <- ggplot()+geom_path(aes(
+    x=life.expectancy, 
+    y=fertility.rate, 
+    color=region,
+    group=country),alpha_off=0.5,
+    data=WorldBank)
+  viz <- list(
+    plot = scatter
+  )
+  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")
+})