Toby rect #178
Toby rect #178
Conversation
|
so with the current code this ggplot http://docs.ggplot2.org/current/geom_rect.html becomes this plotly https://plot.ly/398/~tdhock/ so I guess fill="tozerox" is not good enough when there are multiple rects that I want to draw in 1 trace. Alex said in an email that 'we could always add a fill mode like "toself" tailored for this' @chriddyp do you think I should wait until that is implemented? or I can implement this right now by making each rect a separate trace, as in https://plot.ly/3722/~TestBot/ |
|
haha, well, we tried. |
|
oooookay, it's possible to get this effect with the existing fill modes, but it isn't fun: You need to duplicate the starting corner of each one, then leave a (non-numeric) gap between rects, then when you're done retrace your steps past all the start / end points (including more gaps so the lines don't get drawn), and for some bizarro reason (that's probably a bug but its such a hack at this point I'm not sure I care) you have to duplicate the final point. 💩 If we made a fillmode "toself" tailored to making closed shapes it would:
Anyway, this is all a ways out, it's a bigger project than I can just bang out right now. Until then,if you can stomach my hack, that's going to be the highest performance for a large number of rects, then only make a new trace when you want a new color or some other feature like new dashes as in https://plot.ly/3722/~TestBot/. Maybe also set |
|
@alexcjohnson thanks for the example plotly. It is nice to know your hack will "be the highest performance for a large number of rects" and I think it makes sense to "only make a new trace when you want a new color" so I will go ahead and implement that. |
|
after some experimentation in the plotly GUI, I found that in some cases I don't have to re-trace my steps all the way back to the first trace, but instead just back to the first point of the second trace however indeed in the general case (here with 2 rects) we need to trace back to the first rect and repeat the start position |
|
|
||
| test_that('rect color', { | ||
| info <- expect_traces(rect.color, 2, "rect-color") | ||
| ## TODO: test for weird forward/backward NA pattern with 2 rects. |
There was a problem hiding this comment.
Seems like conversion of geom_polygon and geom_rect is working fine now, but I would like to add a few more tests before merging with master.
| cbind(x=xmax, y=ymin, others)) | ||
| }) | ||
| g$geom <- "polygon" | ||
| g | ||
| }, |
There was a problem hiding this comment.
geom_rect is converted to a basic geom "polygon"
|
|
||
| expect_identical(info[[1]]$showlegend, TRUE) | ||
| expect_identical(info[[2]]$showlegend, TRUE) | ||
| info <- expect_traces(gg, 1, "black") |
There was a problem hiding this comment.
I updated the tests to check for the NA values and retracing the first points of each group.
|
IMO This code is ready to merge with master. Please code review @mkcor @chriddyp @cpsievert |
| expect_identical(tr$type, "scatter") | ||
| expect_identical(tr$mode, "lines") | ||
| for(xy in c("x", "y")){ | ||
| expect_true(any(is.na(tr[[xy]]))) |
There was a problem hiding this comment.
Just FYI, base does have anyNA() which is slightly faster
|
This looks good to me 👍 |
|
for reference here is the test table a new polygon test a new rect test |
I implemented geom_rect conversion by treating it as a basic version of a polygon, which is transformed into a trace with mode="lines", type="scatter", fill="tozerox"