Skip to content

Support geom_vline() conversion #131

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Oct 22, 2014
Merged

Support geom_vline() conversion #131

merged 6 commits into from
Oct 22, 2014

Conversation

mkcor
Copy link
Contributor

@mkcor mkcor commented Oct 22, 2014

  • Also enhanced it for geom_hline() at the same time: You can now pass a vector of values to the intercept parameter, to draw multiple lines (horizontal or vertical).
  • Image diffs can be seen at 08a4199
  • New images are tests/testthat/test-ggplot-vline-*.png, tests/testthat/test-ggplot-vline-multiple-*.png, tests/testthat/test-ggplot-hline-multiple-*.png;
  • tests/cookbook-test-suite/lines/basic_horizontal_line.* has improved a little, but that's because the original ggplot2 code was broken--now fixed, and I'm not supporting x-ranges that are not numerical--to do;
  • @chriddyp hline_on_facets-*.png now fixed (https://plot.ly/~TestBot/690).

/cc @pedrodz

# Split hline and vline when multiple
if (g$geom == "hline" || g$geom == "vline") {
if (nrow(g$data) > 1) {
df.list <- split(basic$data, rep(1:nrow(g$data))) #, drop=TRUE)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, this comment should just go away...

@pedrodz
Copy link
Contributor

pedrodz commented Oct 22, 2014

Great. Fix the comment then :shipit:
Thank you, @mkcor

@chriddyp
Copy link
Member

Nice! Looking through the JSON diffs, and then clicking on the associated images with the same filename was a really nice way to review

@mkcor
Copy link
Contributor Author

mkcor commented Oct 22, 2014

Awesome! Good to know. :)

mkcor added a commit that referenced this pull request Oct 22, 2014
@mkcor mkcor merged commit e53047b into master Oct 22, 2014
@mkcor mkcor deleted the marianne-geom-vline branch October 22, 2014 21:42
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.

3 participants