Skip to content

Commit 0fa47a9

Browse files
cpsievertcpsievert
authored
Merge pull request #1489 from michaelbabyn/dont-src-layout-attributes
dont srcify layout attributes so ticktext works when sending charts to plot.ly
2 parents 8f22d3c + fed2017 commit 0fa47a9

File tree

2 files changed

+156
-150
lines changed

2 files changed

+156
-150
lines changed

‎R/utils.R

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ verify_attr_names <- function(p) {
435435
verify_attr_spec <- function(p) {
436436
if (!is.null(p$x$layout)) {
437437
p$x$layout <- verify_attr(
438-
p$x$layout, Schema$layout$layoutAttributes
438+
p$x$layout, Schema$layout$layoutAttributes, layoutAttr = TRUE
439439
)
440440
}
441441
for (tr in seq_along(p$x$data)) {
@@ -450,7 +450,7 @@ verify_attr_spec <- function(p) {
450450
p
451451
}
452452

453-
verify_attr <- function(proposed, schema) {
453+
verify_attr <- function(proposed, schema, layoutAttr = FALSE) {
454454
for (attr in names(proposed)) {
455455
attrSchema <- schema[[attr]] %||% schema[[sub("[0-9]+$", "", attr)]]
456456
# if schema is missing (i.e., this is an un-official attr), move along
@@ -479,8 +479,10 @@ verify_attr <- function(proposed, schema) {
479479
}
480480

481481
# tag 'src-able' attributes (needed for api_create())
482+
# note that layout has 'src-able' attributes that shouldn't
483+
# be turned into grids https://github.com/ropensci/plotly/pull/1489
482484
isSrcAble <- !is.null(schema[[paste0(attr, "src")]]) && length(proposed[[attr]]) > 1
483-
if (isDataArray || isSrcAble) {
485+
if ((isDataArray || isSrcAble) && !isTRUE(layoutAttr)) {
484486
proposed[[attr]] <- structure(proposed[[attr]], apiSrc = TRUE)
485487
}
486488

@@ -508,7 +510,7 @@ verify_attr <- function(proposed, schema) {
508510

509511
# do the same for "sub-attributes"
510512
if (identical(role, "object") && is.recursive(proposed[[attr]])) {
511-
proposed[[attr]] <- verify_attr(proposed[[attr]], schema[[attr]])
513+
proposed[[attr]] <- verify_attr(proposed[[attr]], schema[[attr]], layoutAttr = layoutAttr)
512514
}
513515
}
514516

‎tests/testthat/test-api.R

Lines changed: 150 additions & 146 deletions
Original file line numberDiff line numberDiff line change
@@ -1,146 +1,150 @@
1-
# context("api")
2-
#
3-
# test_that("api() returns endpoints", {
4-
# skip_on_cran()
5-
# skip_if_not_master()
6-
#
7-
# res <- api()
8-
# expect_true(length(res) > 1)
9-
# expect_true(all(c("plots", "grids", "folders") %in% names(res)))
10-
# })
11-
#
12-
# test_that("Can search with white-space", {
13-
# skip_on_cran()
14-
# skip_if_not_master()
15-
#
16-
# res <- api("search?q=overdose drugs")
17-
# expect_true(length(res) > 1)
18-
# })
19-
#
20-
# test_that("Changing a filename works", {
21-
# skip_on_cran()
22-
# skip_if_not_master()
23-
#
24-
# id <- plotly:::new_id()
25-
# f <- api("files/cpsievert:14680", "PATCH", list(filename = id))
26-
# expect_equivalent(f$filename, id)
27-
# })
28-
#
29-
#
30-
# test_that("Downloading plots works", {
31-
# skip_on_cran()
32-
# skip_if_not_master()
33-
#
34-
# # https://plot.ly/~cpsievert/200
35-
# p <- api_download_plot(200, "cpsievert")
36-
# expect_is(p, "htmlwidget")
37-
# expect_is(p, "plotly")
38-
#
39-
# l <- plotly_build(p)$x
40-
# expect_length(l$data, 1)
41-
#
42-
# # This file is a grid, not a plot https://plot.ly/~cpsievert/14681
43-
# expect_error(
44-
# api_download_plot(14681, "cpsievert"), "grid"
45-
# )
46-
# })
47-
#
48-
#
49-
# test_that("Downloading grids works", {
50-
# skip_on_cran()
51-
# skip_if_not_master()
52-
#
53-
# g <- api_download_grid(14681, "cpsievert")
54-
# expect_is(g, "api_file")
55-
# expect_is(
56-
# tibble::as_tibble(g$preview), "data.frame"
57-
# )
58-
#
59-
# # This file is a plot, not a grid https://plot.ly/~cpsievert/14681
60-
# expect_error(
61-
# api_download_grid(200, "cpsievert"), "plot"
62-
# )
63-
# })
64-
#
65-
#
66-
# test_that("Creating produces a new file by default", {
67-
# skip_on_cran()
68-
# skip_if_not_master()
69-
#
70-
# expect_new <- function(obj) {
71-
# old <- api("folders/home?user=cpsievert")
72-
# new_obj <- api_create(obj)
73-
# Sys.sleep(3)
74-
# new <- api("folders/home?user=cpsievert")
75-
# n <- if (plotly:::is.plot(new_obj)) 2 else 1
76-
# expect_equivalent(old$children$count + n, new$children$count)
77-
# }
78-
#
79-
# expect_new(mtcars)
80-
# # even if plot has multiple traces, only one grid should be created
81-
# p1 <- plot_ly(mtcars, x = ~mpg, y = ~wt)
82-
# p2 <- add_markers(p1, color = ~factor(cyl))
83-
# p3 <- add_markers(p1, color = ~factor(cyl), frame = ~factor(vs))
84-
# expect_new(p1)
85-
# expect_new(p2)
86-
# expect_new(p3)
87-
# })
88-
#
89-
#
90-
# test_that("Can overwrite a grid", {
91-
# skip_on_cran()
92-
# skip_if_not_master()
93-
#
94-
# id <- new_id()
95-
# m <- api_create(mtcars, id)
96-
# m2 <- api_create(iris, id)
97-
# expect_true(identical(m$embed_url, m2$embed_url))
98-
# expect_false(identical(m$cols, m2$cols))
99-
# })
100-
#
101-
# test_that("Can overwrite a plot", {
102-
# skip_on_cran()
103-
# skip_if_not_master()
104-
#
105-
# id <- new_id()
106-
# p <- plot_ly()
107-
# m <- api_create(p, id)
108-
# m2 <- api_create(layout(p, title = "test"), id)
109-
# expect_true(identical(m$embed_url, m2$embed_url))
110-
# expect_false(identical(m$figure$layout$title, m2$figure$layout$title))
111-
# })
112-
#
113-
# test_that("Can create plots with non-trivial src attributes", {
114-
# skip_on_cran()
115-
# skip_if_not_master()
116-
#
117-
# expect_srcified <- function(x) {
118-
# expect_length(strsplit(x, ":")[[1]], 3)
119-
# }
120-
#
121-
# # src-ifies data arrays, but not arrayOk of length 1
122-
# p <- plot_ly(x = 1:10, y = 1:10, marker = list(color = "red"))
123-
# res <- api_create(p)
124-
# trace <- res$figure$data[[1]]
125-
# expect_srcified(trace$xsrc)
126-
# expect_srcified(trace$ysrc)
127-
# expect_true(trace$marker$color == "red")
128-
#
129-
# # can src-ify data[i].marker.color
130-
# p <- plot_ly(x = 1:10, y = 1:10, color = 1:10)
131-
# res <- api_create(p)
132-
# trace <- res$figure$data[[1]]
133-
# expect_srcified(trace$marker$colorsrc)
134-
#
135-
# # can src-ify frames[i].data[i].marker.color
136-
# res <- p %>%
137-
# add_markers(frame = rep(1:2, 5)) %>%
138-
# api_create()
139-
# trace <- res$figure$frames[[1]]$data[[1]]
140-
# expect_srcified(trace$marker$colorsrc)
141-
#
142-
# # can src-ify layout.xaxis.tickvals
143-
# res <- api_create(ggplot() + geom_bar(aes(1:10)))
144-
# expect_srcified(res$figure$layout$xaxis$tickvalssrc)
145-
#
146-
# })
1+
context("api")
2+
3+
test_that("api() returns endpoints", {
4+
skip_on_cran()
5+
skip_if_not_master()
6+
7+
res <- api()
8+
expect_true(length(res) > 1)
9+
expect_true(all(c("plots", "grids", "folders") %in% names(res)))
10+
})
11+
12+
test_that("Can search with white-space", {
13+
skip_on_cran()
14+
skip_if_not_master()
15+
16+
res <- api("search?q=overdose drugs")
17+
expect_true(length(res) > 1)
18+
})
19+
20+
test_that("Changing a filename works", {
21+
skip_on_cran()
22+
skip_if_not_master()
23+
24+
id <- plotly:::new_id()
25+
f <- api("files/cpsievert:14680", "PATCH", list(filename = id))
26+
expect_equivalent(f$filename, id)
27+
})
28+
29+
30+
test_that("Downloading plots works", {
31+
skip_on_cran()
32+
skip_if_not_master()
33+
34+
# https://plot.ly/~cpsievert/200
35+
p <- api_download_plot(200, "cpsievert")
36+
expect_is(p, "htmlwidget")
37+
expect_is(p, "plotly")
38+
39+
l <- plotly_build(p)$x
40+
expect_length(l$data, 1)
41+
42+
# This file is a grid, not a plot https://plot.ly/~cpsievert/14681
43+
expect_error(
44+
api_download_plot(14681, "cpsievert"), "grid"
45+
)
46+
})
47+
48+
49+
test_that("Downloading grids works", {
50+
skip_on_cran()
51+
skip_if_not_master()
52+
53+
g <- api_download_grid(14681, "cpsievert")
54+
expect_is(g, "api_file")
55+
expect_is(
56+
tibble::as_tibble(g$preview), "data.frame"
57+
)
58+
59+
# This file is a plot, not a grid https://plot.ly/~cpsievert/14681
60+
expect_error(
61+
api_download_grid(200, "cpsievert"), "plot"
62+
)
63+
})
64+
65+
66+
test_that("Creating produces a new file by default", {
67+
skip_on_cran()
68+
skip_if_not_master()
69+
70+
expect_new <- function(obj) {
71+
old <- api("folders/home?user=cpsievert")
72+
new_obj <- api_create(obj)
73+
Sys.sleep(3)
74+
new <- api("folders/home?user=cpsievert")
75+
n <- if (plotly:::is.plot(new_obj)) 2 else 1
76+
expect_equivalent(old$children$count + n, new$children$count)
77+
}
78+
79+
expect_new(mtcars)
80+
# even if plot has multiple traces, only one grid should be created
81+
p1 <- plot_ly(mtcars, x = ~mpg, y = ~wt)
82+
p2 <- add_markers(p1, color = ~factor(cyl))
83+
p3 <- add_markers(p1, color = ~factor(cyl), frame = ~factor(vs))
84+
expect_new(p1)
85+
expect_new(p2)
86+
expect_new(p3)
87+
})
88+
89+
90+
test_that("Can overwrite a grid", {
91+
skip_on_cran()
92+
skip_if_not_master()
93+
94+
id <- new_id()
95+
m <- api_create(mtcars, id)
96+
m2 <- api_create(iris, id)
97+
expect_true(identical(m$embed_url, m2$embed_url))
98+
expect_false(identical(m$cols, m2$cols))
99+
})
100+
101+
test_that("Can overwrite a plot", {
102+
skip_on_cran()
103+
skip_if_not_master()
104+
105+
id <- new_id()
106+
p <- plot_ly()
107+
m <- api_create(p, id)
108+
m2 <- api_create(layout(p, title = "test"), id)
109+
expect_true(identical(m$embed_url, m2$embed_url))
110+
expect_false(identical(m$figure$layout$title, m2$figure$layout$title))
111+
})
112+
113+
test_that("Can create plots with non-trivial src attributes", {
114+
skip_on_cran()
115+
skip_if_not_master()
116+
117+
expect_srcified <- function(x) {
118+
expect_length(strsplit(x, ":")[[1]], 3)
119+
}
120+
121+
expect_not_srcified <- function(x) {
122+
expect_true(is.null(x))
123+
}
124+
125+
# src-ifies data arrays, but not arrayOk of length 1
126+
p <- plot_ly(x = 1:10, y = 1:10, marker = list(color = "red"))
127+
res <- api_create(p)
128+
trace <- res$figure$data[[1]]
129+
expect_srcified(trace$xsrc)
130+
expect_srcified(trace$ysrc)
131+
expect_true(trace$marker$color == "red")
132+
133+
# can src-ify data[i].marker.color
134+
p <- plot_ly(x = 1:10, y = 1:10, color = 1:10)
135+
res <- api_create(p)
136+
trace <- res$figure$data[[1]]
137+
expect_srcified(trace$marker$colorsrc)
138+
139+
# can src-ify frames[i].data[i].marker.color
140+
res <- p %>%
141+
add_markers(frame = rep(1:2, 5)) %>%
142+
api_create()
143+
trace <- res$figure$frames[[1]]$data[[1]]
144+
expect_srcified(trace$marker$colorsrc)
145+
146+
# doesn't src-ify layout arrays (layout.xaxis.tickvals)
147+
res <- api_create(ggplot() + geom_bar(aes(1:10)))
148+
expect_not_srcified(res$figure$layout$xaxis$tickvalssrc)
149+
150+
})

0 commit comments

Comments
 (0)