From 6ef95b747540de9744a6d78b59b249dc96981a73 Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 12:12:26 +1000 Subject: [PATCH 1/8] DS-4804 Fixup and add unit test --- R/plotly.R | 2 +- tests/testthat/test-plotly.R | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/R/plotly.R b/R/plotly.R index d67b000722..f6a190c8ba 100644 --- a/R/plotly.R +++ b/R/plotly.R @@ -143,7 +143,7 @@ plot_ly <- function(data = data.frame(), ..., type = NULL, name, if (is.data.frame(data) && nrow(data) > 0L) { qtables <- vapply(data, inherits, logical(1L), c("qTable", "QTable")) if (any(qtables)) - data[qtables] <- lapply(data[qtables], unclass) + data[qtables] <- lapply(data[qtables], as.vector) } # "native" plotly arguments diff --git a/tests/testthat/test-plotly.R b/tests/testthat/test-plotly.R index 7b10220293..6d4b823946 100644 --- a/tests/testthat/test-plotly.R +++ b/tests/testthat/test-plotly.R @@ -358,3 +358,17 @@ test_that("Line breaks are properly translated (R -> HTML)", { test_that("group_by() on a plotly object doesn't produce warning", { expect_warning(group_by(plot_ly(txhousing), city), NA) }) + +test_that("Check QTables dont cause errors", { + .createQTableArray <- function(n, class.name) + structure( + array(runif(n), dim = c(n, 1L)), + class = class.name + ) + s <- data.frame( + x = .createQTableArray(10, "QTable"), + y = .createQTableArray(10, "qTable") + ) + expect_error(p <- plot_ly(s, x = ~x, y = ~y), NA) + expect_error(print(p), NA) +}) From 9603940bc28d6a6c7de94f1806658e91bebeb691 Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 14:17:26 +1000 Subject: [PATCH 2/8] DS-4804 Use CircleCI instead of actions --- .Rbuildignore | 1 + .circleci/config.yml | 52 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 .circleci/config.yml diff --git a/.Rbuildignore b/.Rbuildignore index 63e0d2b162..70ecb6fe2a 100644 --- a/.Rbuildignore +++ b/.Rbuildignore @@ -21,3 +21,4 @@ README.Rmd abbvie.R ^\.httr-oauth$ ^\.github$ +^\.circleci$ diff --git a/.circleci/config.yml b/.circleci/config.yml new file mode 100644 index 0000000000..4bfd631a42 --- /dev/null +++ b/.circleci/config.yml @@ -0,0 +1,52 @@ +version: 2.1 +orbs: + r-packages: displayr/r-packages@dev:alpha +parameters: + trigger-message: + type: string + default: "" + remote-deps: + type: string + default: "" + plugins-branch: + type: string + default: "" + triggered-packages: + type: string + default: "" + executor: + type: enum + enum: [nightly, rocker, machine,rocker-geo] + default: rocker + +workflows: + build-and-check-R-package: + jobs: + - r-packages/build-and-check-package: + executor: << pipeline.parameters.executor >> + name: BuildAndCheckPackage + context: + - r_packages + remote-deps: << pipeline.parameters.remote-deps >> + separate-test-job: false + - r-packages/deploy-package: + executor: << pipeline.parameters.executor >> + requires: + - BuildAndCheckPackage + context: + - r_packages + filters: + branches: + only: + - master + - r-packages/trigger-revdeps: + executor: << pipeline.parameters.executor >> + requires: + - BuildAndCheckPackage + context: + - r_packages + remote-deps: << pipeline.parameters.remote-deps >> + plugins-branch: << pipeline.parameters.plugins-branch >> + trigger-message: << pipeline.parameters.trigger-message >> + triggered-packages: << pipeline.parameters.triggered-packages >> + From 135887daee44084ae0ece2eda6cea64553666b0f Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 14:19:28 +1000 Subject: [PATCH 3/8] DS-4804 Prod CCI From c3259d3910ecb015b4e6ef8000b3064e41cb6fdf Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 14:38:34 +1000 Subject: [PATCH 4/8] DS-4804 Try viz CCI config --- .circleci/config.yml | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 4bfd631a42..a353242699 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -16,37 +16,30 @@ parameters: default: "" executor: type: enum - enum: [nightly, rocker, machine,rocker-geo] - default: rocker + enum: [nightly, rocker, machine.rocker-geo] + default: nightly + save-snapshots: + type: boolean + default: true workflows: build-and-check-R-package: jobs: - r-packages/build-and-check-package: executor: << pipeline.parameters.executor >> + resource-class: small name: BuildAndCheckPackage context: - - r_packages + - rviz_packages remote-deps: << pipeline.parameters.remote-deps >> - separate-test-job: false - - r-packages/deploy-package: + separate-test-job: true + save-snapshots: false + - r-packages/test-package: executor: << pipeline.parameters.executor >> + resource-class: small + name: TestPackage + save-snapshots: << pipeline.parameters.save-snapshots >> requires: - BuildAndCheckPackage context: - - r_packages - filters: - branches: - only: - - master - - r-packages/trigger-revdeps: - executor: << pipeline.parameters.executor >> - requires: - - BuildAndCheckPackage - context: - - r_packages - remote-deps: << pipeline.parameters.remote-deps >> - plugins-branch: << pipeline.parameters.plugins-branch >> - trigger-message: << pipeline.parameters.trigger-message >> - triggered-packages: << pipeline.parameters.triggered-packages >> - + - rviz_packages From c14b8b2cba491a496e9b1930ff8f03251aef44e4 Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 15:39:23 +1000 Subject: [PATCH 5/8] DS-4804 don't split job --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a353242699..d769cd02cd 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -32,7 +32,7 @@ workflows: context: - rviz_packages remote-deps: << pipeline.parameters.remote-deps >> - separate-test-job: true + separate-test-job: false save-snapshots: false - r-packages/test-package: executor: << pipeline.parameters.executor >> From 8088bdc7e26305f7f2256ed2a2ec8224e6d49f97 Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 16:01:12 +1000 Subject: [PATCH 6/8] DS-4804 Remove CCI for now --- .circleci/config.yml | 45 -------------------------------------------- 1 file changed, 45 deletions(-) delete mode 100644 .circleci/config.yml diff --git a/.circleci/config.yml b/.circleci/config.yml deleted file mode 100644 index d769cd02cd..0000000000 --- a/.circleci/config.yml +++ /dev/null @@ -1,45 +0,0 @@ -version: 2.1 -orbs: - r-packages: displayr/r-packages@dev:alpha -parameters: - trigger-message: - type: string - default: "" - remote-deps: - type: string - default: "" - plugins-branch: - type: string - default: "" - triggered-packages: - type: string - default: "" - executor: - type: enum - enum: [nightly, rocker, machine.rocker-geo] - default: nightly - save-snapshots: - type: boolean - default: true - -workflows: - build-and-check-R-package: - jobs: - - r-packages/build-and-check-package: - executor: << pipeline.parameters.executor >> - resource-class: small - name: BuildAndCheckPackage - context: - - rviz_packages - remote-deps: << pipeline.parameters.remote-deps >> - separate-test-job: false - save-snapshots: false - - r-packages/test-package: - executor: << pipeline.parameters.executor >> - resource-class: small - name: TestPackage - save-snapshots: << pipeline.parameters.save-snapshots >> - requires: - - BuildAndCheckPackage - context: - - rviz_packages From 39337c258deba4a4a026b544f6a52288b4205f77 Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 16:37:20 +1000 Subject: [PATCH 7/8] DS-4804 Real fix this time --- R/plotly.R | 1 + tests/testthat/test-plotly.R | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/R/plotly.R b/R/plotly.R index f6a190c8ba..776d037963 100644 --- a/R/plotly.R +++ b/R/plotly.R @@ -140,6 +140,7 @@ plot_ly <- function(data = data.frame(), ..., type = NULL, name, if (!is.data.frame(data) && !crosstalk::is.SharedData(data)) { stop("First argument, `data`, must be a data frame or shared data.", call. = FALSE) } + if (is.data.frame(data) && nrow(data) > 0L) { qtables <- vapply(data, inherits, logical(1L), c("qTable", "QTable")) if (any(qtables)) diff --git a/tests/testthat/test-plotly.R b/tests/testthat/test-plotly.R index 6d4b823946..c573cf2139 100644 --- a/tests/testthat/test-plotly.R +++ b/tests/testthat/test-plotly.R @@ -362,13 +362,18 @@ test_that("group_by() on a plotly object doesn't produce warning", { test_that("Check QTables dont cause errors", { .createQTableArray <- function(n, class.name) structure( - array(runif(n), dim = c(n, 1L)), + array(runif(n), dim = n), class = class.name ) s <- data.frame( x = .createQTableArray(10, "QTable"), - y = .createQTableArray(10, "qTable") + x2 = .createQTableArray(10, "qTable"), + y = factor(letters[1:10]) ) - expect_error(p <- plot_ly(s, x = ~x, y = ~y), NA) + expect_error(p <- plot_ly(s) |> + add_segments(x = ~x, xend = ~x2, y = ~y, yend = ~y, showlegend = FALSE) |> + add_markers(x = ~x, xend = ~y, y = ~y, name = "foo", color = I("orange"), showlegend = FALSE) |> + add_markers(x = ~x2, xend = ~y, y = ~y, name = "bar", color = I("blue"), showlegend = FALSE), + NA) expect_error(print(p), NA) }) From b95fd2ffffc70bd31b90520d5074ddc74e11df2d Mon Sep 17 00:00:00 2001 From: jrwishart Date: Fri, 14 Jul 2023 16:58:29 +1000 Subject: [PATCH 8/8] DS-4804 Allow test to work on earlier R versions --- tests/testthat/test-plotly.R | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/testthat/test-plotly.R b/tests/testthat/test-plotly.R index c573cf2139..979a9a8296 100644 --- a/tests/testthat/test-plotly.R +++ b/tests/testthat/test-plotly.R @@ -360,16 +360,13 @@ test_that("group_by() on a plotly object doesn't produce warning", { }) test_that("Check QTables dont cause errors", { - .createQTableArray <- function(n, class.name) - structure( - array(runif(n), dim = n), - class = class.name - ) s <- data.frame( - x = .createQTableArray(10, "QTable"), - x2 = .createQTableArray(10, "qTable"), + x = array(runif(n), dim = n), + x2 = array(runif(n), dim = n), y = factor(letters[1:10]) ) + class(s[[1]]) <- "QTable" + class(s[[2]]) <- "qTable" expect_error(p <- plot_ly(s) |> add_segments(x = ~x, xend = ~x2, y = ~y, yend = ~y, showlegend = FALSE) |> add_markers(x = ~x, xend = ~y, y = ~y, name = "foo", color = I("orange"), showlegend = FALSE) |>