From 87ab80f76841e465761d235f2be0ab58b598cd7e Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Thu, 14 Dec 2023 10:51:38 -0700 Subject: [PATCH 1/4] test for space free vertical --- tests/testthat/test-renderer1-facet-space.R | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-renderer1-facet-space.R b/tests/testthat/test-renderer1-facet-space.R index 2033e6037..faee446aa 100644 --- a/tests/testthat/test-renderer1-facet-space.R +++ b/tests/testthat/test-renderer1-facet-space.R @@ -12,7 +12,13 @@ viz <- animint( facet_grid(.~am, scales="free", labeller=label_both), fixed = no.panels + ggtitle("space=fixed, scales=fixed")+ - facet_grid(.~am, labeller=label_both)) + facet_grid(.~am, labeller=label_both), + freeBothFlip = ggplot(mtcars, aes(wt, mpg)) + + ggtitle("space=free, scales=free") + + scale_x_continuous(breaks=seq(1, 9)) + + geom_point(colour='grey50', size = 4) + + geom_point(aes(colour = cyl)) + + facet_grid(am ~ vs, space = "free", scales = "free", labeller=label_both)) info <- animint2HTML(viz) ## For some reason the "space between panels" tests only work on @@ -91,6 +97,18 @@ test_that("pixels between 15 and 20 is constant or variable", { expect_equal(length(x.axes), 2) xdiff <- lapply(x.axes, getTickDiff) expect_true(both.equal(xdiff)) + ## scale="free" and space="free" means the distance between ticks + ## should be the same across the horizontal and vertical panels. + x.axes <- getNodeSet( + info$html, "//svg[@id='plot_freeBothFlip']//g[contains(@class, 'xaxis')]") + expect_equal(length(x.axes), 2) + xdiff <- lapply(x.axes, getTickDiff, axis="x") + expect_true(both.equal(xdiff)) + y.axes <- getNodeSet( + info$html, "//svg[@id='plot_freeBothFlip']//g[contains(@class, 'yaxis')]") + expect_equal(length(y.axes), 2) + ydiff <- lapply(y.axes, getTickDiff, axis="y") + expect_true(both.equal(ydiff)) }) test_that("width_proportion is constant or variable", { From 26d1df797bb35267b34a4c84412fc63ba189c904 Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Thu, 14 Dec 2023 11:17:45 -0700 Subject: [PATCH 2/4] lookup range using panel value (not scale) --- R/z_facets.R | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/R/z_facets.R b/R/z_facets.R index 59abd320f..78d6999cb 100644 --- a/R/z_facets.R +++ b/R/z_facets.R @@ -114,14 +114,17 @@ train_layout <- function(facet, coord, layout, ranges) { scale.type <- paste0("SCALE_", u.type) range.type <- paste0(l.type, ".range") space.type <- paste0("SPACE_", u.type) - vals <- layout[[scale.type]] - uv <- unique(vals) - diffs <- sapply(ranges[uv], function(x) { - diff(x[[range.type]]) + scale.vals <- layout[[scale.type]] + uniq.scale.vals <- unique(scale.vals) + diffs <- sapply(uniq.scale.vals, function(scale.value) { + panel.value <- which(scale.vals==scale.value)[1] + diff(ranges[[panel.value]][[range.type]]) }) # decide the proportion of the height/width each scale deserves based on the range - props <- data.frame(tmp1 = uv, tmp2 = diffs / sum(diffs)) - names(props) <- c(scale.type, space.type) + props.list <- list() + props.list[[scale.type]] <- uniq.scale.vals + props.list[[space.type]] <- diffs / sum(diffs) + props <- do.call(data.frame, props.list) layout <- plyr::join(layout, props, by = scale.type) } names(layout) <- gsub("SPACE_X", "width_proportion", names(layout), fixed = TRUE) From 9fb0901a2db31c956e1b6e054cd1f4574bac021e Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Thu, 14 Dec 2023 11:20:15 -0700 Subject: [PATCH 3/4] version++ --- DESCRIPTION | 2 +- NEWS.md | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index c43813020..1113437fa 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: animint2 Title: Animated Interactive Grammar of Graphics -Version: 2023.11.21 +Version: 2023.12.14 URL: https://animint.github.io/animint2/ BugReports: https://github.com/animint/animint2/issues Authors@R: c( diff --git a/NEWS.md b/NEWS.md index 70ce4ba5b..d919654b1 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,7 @@ +# Changes in version 2023.12.14 (PR#112) + +- bugfix in compiler height_proportion computation, which occured in ggplots with space=free and both vertical/horizontal panels. Before the vertical panels were always the same size, now they can be different sizes. + # Changes in version 2023.11.21 - setDTthreads(1) in CRAN testthat.R (created from build.sh). From df90a3f7619dedeb03c0fc69564e40d68d9b8be4 Mon Sep 17 00:00:00 2001 From: Toby Dylan Hocking Date: Thu, 14 Dec 2023 11:33:17 -0700 Subject: [PATCH 4/4] move new test to separate viz --- tests/testthat/test-renderer1-facet-space.R | 31 ++++++++++++--------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/tests/testthat/test-renderer1-facet-space.R b/tests/testthat/test-renderer1-facet-space.R index faee446aa..4f56feb4f 100644 --- a/tests/testthat/test-renderer1-facet-space.R +++ b/tests/testthat/test-renderer1-facet-space.R @@ -12,13 +12,7 @@ viz <- animint( facet_grid(.~am, scales="free", labeller=label_both), fixed = no.panels + ggtitle("space=fixed, scales=fixed")+ - facet_grid(.~am, labeller=label_both), - freeBothFlip = ggplot(mtcars, aes(wt, mpg)) + - ggtitle("space=free, scales=free") + - scale_x_continuous(breaks=seq(1, 9)) + - geom_point(colour='grey50', size = 4) + - geom_point(aes(colour = cyl)) + - facet_grid(am ~ vs, space = "free", scales = "free", labeller=label_both)) + facet_grid(.~am, labeller=label_both)) info <- animint2HTML(viz) ## For some reason the "space between panels" tests only work on @@ -97,6 +91,23 @@ test_that("pixels between 15 and 20 is constant or variable", { expect_equal(length(x.axes), 2) xdiff <- lapply(x.axes, getTickDiff) expect_true(both.equal(xdiff)) +}) + +test_that("width_proportion is constant or variable", { + expect_true(both.equal(info$plots$fixed$layout$width_proportion)) + expect_true(both.equal(info$plots$freeScale$layout$width_proportion)) + expect_true(!both.equal(info$plots$freeBoth$layout$width_proportion)) +}) + +viz <- animint( + freeBothFlip = ggplot(mtcars, aes(wt, mpg)) + + ggtitle("space=free, scales=free") + + scale_x_continuous(breaks=seq(1, 9)) + + geom_point(colour='grey50', size = 4) + + geom_point(aes(colour = cyl)) + + facet_grid(am ~ vs, space = "free", scales = "free", labeller=label_both)) +info <- animint2HTML(viz) +test_that("same spacing for vertical and horizontal ticks", { ## scale="free" and space="free" means the distance between ticks ## should be the same across the horizontal and vertical panels. x.axes <- getNodeSet( @@ -111,12 +122,6 @@ test_that("pixels between 15 and 20 is constant or variable", { expect_true(both.equal(ydiff)) }) -test_that("width_proportion is constant or variable", { - expect_true(both.equal(info$plots$fixed$layout$width_proportion)) - expect_true(both.equal(info$plots$freeScale$layout$width_proportion)) - expect_true(!both.equal(info$plots$freeBoth$layout$width_proportion)) -}) - no.panels <- ggplot(mtcars, aes(wt, mpg)) + geom_point(colour='grey50', size = 4) + geom_point(aes(colour = cyl))