Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 0fa47a9

Browse filesBrowse 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
Copy full SHA for 0fa47a9

File tree

Expand file treeCollapse file tree

2 files changed

+156
-150
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+156
-150
lines changed

‎R/utils.R

Copy file name to clipboardExpand all lines: R/utils.R
+6-4Lines 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

Copy file name to clipboard
+150-146Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.