From c40657a0884452836ec0c3fd556f7ac956ed4e82 Mon Sep 17 00:00:00 2001
From: Stefan Moog <moogs@gmx.de>
Date: Sat, 31 Aug 2024 15:33:07 +0200
Subject: [PATCH 1/3] Fix. Add special treatment of `dimensions` attrbute to
account for the data_array propery of the "values" attribute. Closes #2385.
* Add tests to check that "values" property has class "AsIs" for "parcoords", "parcats" and "splom" traces.
* Add NEWS entry
---
NEWS.md | 2 ++
R/utils.R | 10 +++++++++-
tests/testthat/test-plotly-parcats.R | 20 ++++++++++++++++++++
tests/testthat/test-plotly-parcoords.R | 20 ++++++++++++++++++++
tests/testthat/test-plotly-splom.R | 22 ++++++++++++++++++++--
5 files changed, 71 insertions(+), 3 deletions(-)
create mode 100644 tests/testthat/test-plotly-parcats.R
create mode 100644 tests/testthat/test-plotly-parcoords.R
diff --git a/NEWS.md b/NEWS.md
index fed423fe24..cc6373aeb8 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -12,6 +12,8 @@
* Closed #2337: Creating a new `event_data()` handler no longer causes a spurious reactive update of existing `event_data()`s. (#2339)
+* Closed #2385: `"parcoords"`, `"parcats"` and `"splom"` traces now work with one-dimensional data by accounting for the "data_array" property of the "values" attribute of "dimensions".
+
# 4.10.4
## Improvements
diff --git a/R/utils.R b/R/utils.R
index da79d43cf2..3705fc1eb6 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -551,7 +551,15 @@ verify_attr <- function(proposed, schema, layoutAttr = FALSE) {
# do the same for "sub-attributes"
if (identical(role, "object") && is.recursive(proposed[[attr]])) {
- proposed[[attr]] <- verify_attr(proposed[[attr]], attrSchema, layoutAttr = layoutAttr)
+ # The "dimensions" attribute requires a special treatment as
+ # it is an unnamed list and hence will be skipped by `for (attr in names(proposed))
+ if (attr == "dimensions") {
+ proposed[[attr]] <- lapply(proposed[[attr]], \(x) {
+ verify_attr(x, attrSchema$items$dimension, layoutAttr = layoutAttr)
+ })
+ } else {
+ proposed[[attr]] <- verify_attr(proposed[[attr]], attrSchema, layoutAttr = layoutAttr)
+ }
}
}
diff --git a/tests/testthat/test-plotly-parcats.R b/tests/testthat/test-plotly-parcats.R
new file mode 100644
index 0000000000..7477bbfc09
--- /dev/null
+++ b/tests/testthat/test-plotly-parcats.R
@@ -0,0 +1,20 @@
+expect_traces <- function(p, n.traces, name) {
+ stopifnot(is.numeric(n.traces))
+ L <- expect_doppelganger_built(p, paste0("plotly-", name))
+ expect_equivalent(length(L$data), n.traces)
+ L
+}
+
+test_that("values property has a class of AsIs", {
+ p <- plot_ly(
+ dimensions = list(
+ list(values = "A"),
+ list(values = "B")
+ ),
+ type = "parcats"
+ )
+ l <- expect_traces(p, 1, "parcats-data-array")
+ tr <- l$data[[1]]$dimensions
+ expect_true(inherits(tr[[1]]$values, "AsIs"))
+ expect_true(inherits(tr[[2]]$values, "AsIs"))
+})
diff --git a/tests/testthat/test-plotly-parcoords.R b/tests/testthat/test-plotly-parcoords.R
new file mode 100644
index 0000000000..180b466903
--- /dev/null
+++ b/tests/testthat/test-plotly-parcoords.R
@@ -0,0 +1,20 @@
+expect_traces <- function(p, n.traces, name) {
+ stopifnot(is.numeric(n.traces))
+ L <- expect_doppelganger_built(p, paste0("plotly-", name))
+ expect_equivalent(length(L$data), n.traces)
+ L
+}
+
+test_that("values property has a class of AsIs", {
+ p <- plot_ly(
+ dimensions = list(
+ list(label = "A", values = 3),
+ list(label = "B", values = 8)
+ ),
+ type = "parcoords"
+ )
+ l <- expect_traces(p, 1, "parcoords-data-array")
+ tr <- l$data[[1]]$dimensions
+ expect_true(inherits(tr[[1]]$values, "AsIs"))
+ expect_true(inherits(tr[[2]]$values, "AsIs"))
+})
diff --git a/tests/testthat/test-plotly-splom.R b/tests/testthat/test-plotly-splom.R
index 5580bff1ee..ef13df0aaa 100644
--- a/tests/testthat/test-plotly-splom.R
+++ b/tests/testthat/test-plotly-splom.R
@@ -1,5 +1,9 @@
-
-
+expect_traces <- function(p, n.traces, name) {
+ stopifnot(is.numeric(n.traces))
+ L <- expect_doppelganger_built(p, paste0("plotly-", name))
+ expect_equivalent(length(L$data), n.traces)
+ L
+}
test_that("No cartesian axes are supplied to a splom chart", {
@@ -12,3 +16,17 @@ test_that("No cartesian axes are supplied to a splom chart", {
)
})
+
+test_that("values property has a class of AsIs", {
+ p <- plot_ly(
+ type = "splom",
+ dimensions = list(
+ list(values = 1, label = "A"),
+ list(values = 2, label = "B")
+ )
+ )
+ l <- expect_traces(p, 1, "parcats-data-array")
+ tr <- l$data[[1]]$dimensions
+ expect_true(inherits(tr[[1]]$values, "AsIs"))
+ expect_true(inherits(tr[[2]]$values, "AsIs"))
+})
From 80a3e5e4e6eb705547fe0870849d957d786aba3e Mon Sep 17 00:00:00 2001
From: Stefan Moog <moogs@gmx.de>
Date: Fri, 6 Sep 2024 12:12:13 +0200
Subject: [PATCH 2/3] Apply suggestions from code review
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
---
R/utils.R | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/R/utils.R b/R/utils.R
index 3705fc1eb6..c8cac21483 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -551,14 +551,18 @@ verify_attr <- function(proposed, schema, layoutAttr = FALSE) {
# do the same for "sub-attributes"
if (identical(role, "object") && is.recursive(proposed[[attr]])) {
- # The "dimensions" attribute requires a special treatment as
- # it is an unnamed list and hence will be skipped by `for (attr in names(proposed))
- if (attr == "dimensions") {
- proposed[[attr]] <- lapply(proposed[[attr]], \(x) {
- verify_attr(x, attrSchema$items$dimension, layoutAttr = layoutAttr)
- })
+ # Some attributes (e.g., dimensions, transforms) are actually
+ # a list of objects (even though they, confusingly, have role: object)
+ # In those cases, we actually want to verify each list element
+ attr_ <- sub("s$", "", attr)
+ is_list_attr <- ("items" %in% names(attrSchema)) &&
+ (attr_ %in% names(attrSchema$items))
+ if (is_list_attr) {
+ proposed[[attr]] <- lapply(proposed[[attr]], function(x) {
+ verify_attr(x, attrSchema$items[[attr_]])
+ })
} else {
- proposed[[attr]] <- verify_attr(proposed[[attr]], attrSchema, layoutAttr = layoutAttr)
+ proposed[[attr]] <- verify_attr(proposed[[attr]], attrSchema, layoutAttr = layoutAttr)
}
}
}
From bb48de83b0cb3bae3866d26018f14e799d7a485f Mon Sep 17 00:00:00 2001
From: Stefan Moog <moogs@gmx.de>
Date: Fri, 6 Sep 2024 12:13:06 +0200
Subject: [PATCH 3/3] Update NEWS.md
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
---
NEWS.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/NEWS.md b/NEWS.md
index cc6373aeb8..ddda02d1af 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -12,7 +12,7 @@
* Closed #2337: Creating a new `event_data()` handler no longer causes a spurious reactive update of existing `event_data()`s. (#2339)
-* Closed #2385: `"parcoords"`, `"parcats"` and `"splom"` traces now work with one-dimensional data by accounting for the "data_array" property of the "values" attribute of "dimensions".
+* Closed #2385: `"parcoords"`, `"parcats"` and `"splom"` traces now work when `values` is a length 1 vector (also, more generally, attributes that expect a list of objects should have those objects "verified" correctly now).
# 4.10.4