Skip to content

Conversation

@cpsievert
Copy link
Collaborator

This will close #179.

@cpsievert
Copy link
Collaborator Author

@chriddyp
Copy link
Member

@cpsievert nice! yes, let's add the scatter plots cookbook file to this PR, as it contains a bunch of really good examples

@tdhock
Copy link
Contributor

tdhock commented Mar 10, 2015

great, @cpsievert can you add your name to the table in the comment I just made on #161 ?

@chriddyp
Copy link
Member

on another note, it seems like the color mapping is off:
ggplot2:
image
plotly:
image

@cpsievert
Copy link
Collaborator Author

This appears to be mostly working now. Only caveat is the "color" legend.

http://ropensci.github.io/plotly-test-table/tables/91e6e3d6dc473e62a6ec2b67a46d29460adc3fca/scatterplots-scale-color-hue.html

Is it possible to merge points and lines in a legend with plotly like ggplot? If so, anyone have a good example? @tdhock @chriddyp

@chriddyp
Copy link
Member

cool, looks good! we don't have the notion of merged legend items. for this case, we could "hide" the other legend items use the trace property "showlegend"

@cpsievert
Copy link
Collaborator Author

@chriddyp
Copy link
Member

looks good 👍 ! @tdhock - will you do a code review too?

R/ggplotly.R Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is confusing to use lapply with an anonymous function here where you could have used a for loop.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tdhock What?! I would say that, in R, you want to use *ply functions (in order to avoid for loops) as much as possible!!

@cpsievert
Copy link
Collaborator Author

I managed to avoid geom specific code in ggplotly.R via a recursive call to layer2traces(). The test table looks good. Let me know what you think @tdhock

http://ropensci.github.io/plotly-test-table/tables/a97ba2b6b71e3f335099d9e73cc13ea18a13e14e/index.html

@tdhock
Copy link
Contributor

tdhock commented Mar 12, 2015

+1 thanks for making that change!

Everything looks good to me, in the code and on the test table.

cpsievert added a commit that referenced this pull request Mar 12, 2015
@cpsievert cpsievert merged commit 09f9dac into master Mar 12, 2015
@cpsievert cpsievert deleted the carson-smooth branch March 12, 2015 22:06
@cpsievert cpsievert restored the carson-smooth branch March 19, 2015 20:00
@cpsievert cpsievert deleted the carson-smooth branch March 19, 2015 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

R API: ggplot2 ggplotly(): geom_smooth() not supported

5 participants