Skip to content

update to plotly 1.11.0 #586

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 3 commits into from
May 18, 2016
Merged

update to plotly 1.11.0 #586

merged 3 commits into from
May 18, 2016

Conversation

timelyportfolio
Copy link
Collaborator

update to plotly 1.11.0

@cpsievert
Copy link
Collaborator

@timelyportfolio do you think it makes sense to reflect plotly.js' semantic versioning in the R package? In other words, since plotly.sj bumped from 1.10.1 to 1.11.0, should we bump the R package from 3.6.0 to 3.7.0?

@timelyportfolio
Copy link
Collaborator Author

@cpsievert I was just wondering that. It makes sense, but compare the big subplot bump to this little puny one. To the R user, little changes. I vote for just 0.0.1 bump, but I could argue either way :)

@cpsievert
Copy link
Collaborator

yea, that has been my approach thus far, so let's stick with that.

Go ahead and bump the version, update news, and you have my 💃

@timelyportfolio timelyportfolio merged commit f4852b9 into master May 18, 2016
@talgalili
Copy link
Contributor

My two cents (which basically supports your positions):

I follow http://semver.org/ where it says that given a version number MAJOR.MINOR.PATCH, increment the:
MAJOR version when you make incompatible API changes,
MINOR version when you add functionality in a backwards-compatible manner, and
PATCH version when you make backwards-compatible bug fixes.

I like this distinction between functionality and bug fixes.
So, from my perspective, if plotly.js upgrade mainly dealt with bug fixes it should be a PATCH bump, but if it dealt with new functionality (such as with the recent subplot) - then MINOR bump should be made (with update to NEWS on what is new for the R user).

Thank you both for the great support of this package!

Tal

@talgalili
Copy link
Contributor

Following my previous note. I am looking at the release notes of plotly 1.11.0:
https://github.com/plotly/plotly.js/releases/tag/v1.11.0
And it says:
Add top-level methods Plotly.toImage to convert a plotly graph to an image data URL (svg, png, jpg, and webp are supported) and Plotly.downloadImage to download a plotly graph as an image [#446]
Add the ability to add arbitrary images loaded from a url to a plot's layout [#525]
Add the option of making legend span horizontally [#535]

If this is also reflected in the R options, then I would not consider this a patch addition, this is new (and useful!) functionality.

Tal

@timelyportfolio
Copy link
Collaborator Author

Good point @talgalili on bumping minor on support of the new functionality in R. None have been implemented on the R side yet. Any in particular appeal to you?

@talgalili
Copy link
Contributor

Thanks @timelyportfolio
Yes, the following (by order) are (IMHO) important to add:

this is nice, it enable implementing "annotation_custom".
For example:

# source: http://stackoverflow.com/questions/9917049/inserting-an-image-to-ggplot2
library(png)
library(grid)
library(ggplot2)
img <- readPNG(system.file("img", "Rlogo.png", package="png"))
g <- rasterGrob(img, interpolate=TRUE)

p <- qplot(1:10, 1:10, geom="blank") +
  annotation_custom(g, xmin=-Inf, xmax=Inf, ymin=-Inf, ymax=Inf) +
  geom_point()

p

library(plotly)
ggplotly(p) # currently doesn't show the figure

But also annotation_raster:

# source: http://stackoverflow.com/questions/27637455/display-custom-image-as-geom-point
library(png)
library(ggplot2)

img <- readPNG(system.file("img", "Rlogo.png", package="png"))

p <- ggplot(mtcars, aes(mpg, wt)) + 
       mapply(function(xx, yy) 
          annotation_raster(img, xmin=xx-1, xmax=xx+1, ymin=yy-0.2, ymax=yy+0.2),
          mtcars$mpg, mtcars$wt) 


p

library(plotly)
ggplotly(p) # doesn't work

@cpsievert cpsievert deleted the update/plotlyjs-1.11.0 branch August 22, 2016 14:41
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