plotly / plotly.R

An interactive graphing library for R
https://plotly-r.com
Other
2.55k stars 625 forks source link

Proposed re-design #40

Closed chriddyp closed 9 years ago

chriddyp commented 10 years ago

With the configuration files we can re-architect the module to be a little more "R"-y. I propose the following interface:

Before

py = plotly(username, key)
trace0 = list(x=...y=...)
py$plotly(trace0)

After

plotly:::set_credentials(username, key)
plotly:::plot(trace0)

Or, if you prefer to send your credentials on a per-call basis (e.g. you're using the R demo account):

plotly:::plot(trace0, username=..., key=....)

And similarly for the other functions:

plotly:::ggplotly(..., plotly_options)
plotly:::plot(..., plotly_options)
plotly:::export_image(..., plotly_options)
plotly:::get_figure(..., plotly_options)

With additional functionality ...

figure = plotly:::get_figure(file_owner, file_id, plotly_options) # returns a list of lists of lists of ... (plotly's JSON graph JSON)

data = plotly:::get_data(figure) # doesn't make a request to plotly, it just removes strips keys out of the named list

A few questions I have (because I'm not an R expert)

1 - Is it the R convention to have nested modules/functions?

e.g. in Python we do:

import plotly.plotly as py

# all the functions in py, call the plotly servers
py.plot(...)
py.iplot(...)
py.get_figure(...)
py.image.save_as(...)

# other functions, like tools, are in a separate namespace:

```python
import plotly.tools as tls
tls.embed(figure)

Do we do a similar thing in R? Or do we keep all the functions at the same "level", e.g.

This:

plotly:::embed(figure)

or this:

plotly:::tools:::embed(figure) #??
tdhock commented 10 years ago

Some constructive criticism:

"if you prefer to send your credentials on a per-call basis" --- that would be really inconvenient for me, who do you think would do that?

personally I prefer the existing interface, where the ~/.Rprofile file itself can be used instead of a credentials file. I write my plotly() call inside

py <- plotly("USER", "PASS")

Then later I will call py$ggplotly(). But I do understand the desire to have a common credentials file if you will be using several client APIs on the same machine.

In R all functions are on the same level:

plotly:::embed(figure)
tdhock commented 10 years ago

An argument for your redesign is that it should be easier for debugging and development.

Developing/testing/debugging with "Before" means executing

source("plotly.R")
source("ggplotly.R")
py <- plotly("USER", "PASS")
py$ggplotly()

whereas with "After" it is 1 line shorter (assuming default user/pass taken from credentials file)

source("plotly.R")
source("ggplotly.R")
ggplotly()

currently instead of re-making the py object every time, I use Plotly/test_ggplotly defined as follows in my ~/.Rprofile

  Plotly <- plotly("tdhock", "PASS")
  test_ggplotly <- function(gg, p=Plotly){
    if(!is.ggplot(gg)){
      stop("gg must be a ggplot")
    }
    if(!is.function(p$plotly)){
      stop("p must be a plotly interface object")
    }
    pargs <- gg2list(gg)
    resp <- do.call(p$plotly, pargs)
    browseURL(resp$url)
    invisible(list(data=pargs, response=resp))
  }

and my testing/dev command lines look like

source("ggplotly.R")
test_ggplotly(some_ggplot)
chriddyp commented 10 years ago

My main argument for:

plotly::plot(data)

instead of

py<-plotly()
py$plot(data)

is that:

1 - Users will set-up their config once (like they do with git/.gitconfig). After they've set-up their config, I think it is unclear for people what py<-plotly() does, and why it's needed! 2 - It seems (but maybe I'm off here, I'm pretty new to R) that the list$function syntax is unconventional/not used often?

mkcor commented 10 years ago

@tdhock We spoke yesterday, so this is for reference only. ;) Reply to your constructive criticism:

py <- plotly()

retrieves the credentials from your global Plotly configuration (let's call them your default credentials).

You may want to use certain credentials on a per-call basis (different from the default credentials). Use cases include: You use more than one Plotly account, where one is public, and/or one is private, and/or one is personal, and/or one is shared with other people, and/or one is for demos, testing, a specific project, etc.

py2 <- plotly("otherAccount", "otherKey")
tdhock commented 10 years ago

Chris you are right that it is not obvious why one would need to type py<-plotly().

With that in mind I think it is a good idea to do the re-deisgn.

However please do not use plotly::plot(data) -- it is problematic to define R functions in a package that have the same name as base R functions. Just call the main plotly function something else other than "plot"

On Wed, May 28, 2014 at 3:27 PM, Marianne Corvellec < notifications@github.com> wrote:

@tdhock https://github.com/tdhock We spoke yesterday, so this is for reference only. ;) Reply to your constructive criticism:

py <- plotly()

retrieves the credentials from your global Plotly configuration (let's call them your default credentials).

You may want to use certain credentials on a per-call basis (different from the default credentials). Use cases include: You use more than one Plotly account, where one is public, and/or one is private, and/or one is personal, and/or one is shared with other people, and/or one is for demos, testing, a specific project, etc.

— Reply to this email directly or view it on GitHubhttps://github.com/ropensci/plotly/issues/40#issuecomment-44453120 .

vdimarco commented 10 years ago

@tdhock regarding your comment:

However please do not use plotly::plot(data) -- it is problematic to define R functions in a package that have the same name as base R functions. Just call the main plotly function something else other than "plot"

Do namespaces solve this problem?

tdhock commented 10 years ago

Defining a plotly::plot function is not a technical problem; it is indeed possible.

It is a user interface problem -- I think it is confusing to use the same function name as the base plot function.

cpsievert commented 9 years ago

I agree with @chriddyp that the current interface is confusing. As I understand it, the main reason for having a function return a list of functions is to enable a mutable state. As far I can tell, that isn't necessary here.

In addition to avoiding permission issues when reading/writing credentials to disk, if we kept track of them via environment variables, you could still specify non-default credential on a per-call basis:

plotly <- function(p, username, key, ...) {
  if (missing(username)) username <- Sys.getenv("PLOTLY_USER")
  if (missing(username)) username <- Sys.getenv("PLOTLY_KEY")

  ....
}

Then you could just do

# use default credentials
plotly::plotly(qplot(1:10))

or

plotly::plotly(qplot(1:10), "my_username", "my_key")

I'd be interested in helping with a redesign, but the changes would be backwards-incompatible and it might be a lot of work.

Also, @chriddyp, I don't think nested namespaces are possible in R. However, it's worth noting that with knitr, we can control printing of objects based on their class. We do this in animint. For example,

```r
(p <- qplot(1:10))

would embed a ggplot2 plot
structure(p, class = "animint")


calls `animint2dir` and embeds the animint plot