Skip to content

Conversation

@pedrodz
Copy link
Contributor

@pedrodz pedrodz commented Oct 22, 2014

Summary:

  • Fix typo in line style mapping (dotted -> dot);
  • Add multiplier (2x) for converting line size from ggplot2 to line width in Plotly;
  • Fix some tests. For example, they were using dotted line type - there's no such thing in Plotly; and,
  • Some styling.

Line width default was 2, that means when line size was not defined, an 1 in ggplot2 was a 2 in Plotly. Now when size was defined it had the same value in Plotly, when it should be 2x - a 1.5 in ggplot2 should be a 3 in Plotly.
ggplot2: http://www.cookbook-r.com/Graphs/Bar_and_line_graphs_%28ggplot2%29/line-red-dotted-shape.png):
Plotly before: https://plot.ly/~pdespouy/1807
Plotly after: https://plot.ly/~pdespouy/1806

chriddyp and others added 29 commits October 15, 2014 23:54
…otly into add-r-cookbook-tests

git push was conflicting. Include Marianne's changes
@pedrodz
Copy link
Contributor Author

pedrodz commented Oct 23, 2014

Images can be find here: a6d3230. There's a lot of plots that don't look good, but they were fixed with the following commits: 529a277 and 563b0c4. Bottom line, the only image that changed is tests/testthat/test-ggplot-abline-plotly.png, where the markers size got fixed with commit 529a277 - see 64e75ae.

/cc @mkcor

@chriddyp
Copy link
Member

Looking good! Is it possible to see how the cookbook examples changed?

@pedrodz
Copy link
Contributor Author

pedrodz commented Oct 25, 2014

Good idea. OK, retrieved the plots that changed: 275f6ad (lines.R) and bfaf60e (bars_and_lines.R)
Thank you, @chriddyp!

@pedrodz
Copy link
Contributor Author

pedrodz commented Oct 25, 2014

@chriddyp
Copy link
Member

Looking good!

@chriddyp
Copy link
Member

💃

pedrodz added a commit that referenced this pull request Oct 25, 2014
@pedrodz pedrodz merged commit e9fe85d into master Oct 25, 2014
@pedrodz pedrodz deleted the pd-line-styles-and-width branch October 28, 2014 17:02
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.

4 participants