ggplot import support about shinystan

[dpastoor]

theme elements done in c2b68cb

[jgabry]

Cool. Let's do the rest of these in the update_ggplot_elements branch.

[jgabry]

Actually @dpastoor, instead of putting ggplot2:: everywhere in the code, we might just be able to put a bunch of things like this:

geom_histogram <- ggplot2::geom_histogram
element_line <- ggplot2::element_line
qplot <- ggplot2::qplot
etc.

in a single file that gets loaded before any of these functions are used. Or am I overlooking something?

[bob-carpenter]

As long as it's in a local scope so this doesn't just reintroduce
name conflicts.

• Bob

On Aug 12, 2015, at 5:45 PM, Jonah Gabry [email protected] wrote:

Actually @dpastoor, instead of putting ggplot2:: everywhere in the code, we might just be able to put a bunch of things like this:

geom_histogram <- ggplot2::geom_histogram
element_line <- ggplot2::element_line
qplot <- ggplot2::qplot
etc.

in a single file that gets loaded before any of these functions are used. Or am I overlooking something?

—
Reply to this email directly or view it on GitHub.

[jgabry]

Yeah this would be in a local scope and not carry over into the user's R
session when the GUI is closed.

On Wednesday, August 12, 2015, Bob Carpenter [email protected]
wrote:

As long as it's in a local scope so this doesn't just reintroduce
name conflicts.

• Bob

On Aug 12, 2015, at 5:45 PM, Jonah Gabry <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> wrote:

Actually @dpastoor, instead of putting ggplot2:: everywhere in the code,
we might just be able to put a bunch of things like this:

geom_histogram <- ggplot2::geom_histogram
element_line <- ggplot2::element_line
qplot <- ggplot2::qplot
etc.

in a single file that gets loaded before any of these functions are
used. Or am I overlooking something?

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub
#85 (comment).

[jgabry]

@dpastoor I'll see if my strategy works and let you know. I think it would be preferable to not have a zillion ggplot2::fun's all over the place that make the source code really hard to read.

[dpastoor]

@jgabry the problem I have with that is you now have to track any possible ggplot element you use across the package, so any time you want to add something you have to check to say hmm do all the elements I call here actually exist in, shall we call it, ggplot 'local', so any new function you'd need to check the imports list so to speak and you'll end up with a mess of

aes <- ggplot::aes
scale_color_manual
scale_color_discrete
....

On top of that, I don't know if the CRAN people will go for that, I seem to recall reading something about them frowning heavily on that kind of 'ghetto import'.

But I absolutely agree that it will also be a bit of mess to find every call and enforce ggplot::, likewise, it will be tedious actually writing the code.

What about a sort of middle ground:

See if we can define the entry points to where ggplot components may be used in a standalone fashion in another package, maybe that is just launch_shinystan, but there could be some other functions that would be useable outside shinystan.

Then all we have to do is add the snippet i mentioned in #82 to explicitly load ggplot in those. Inside of shinystan itself that'll always return false since ggplot is attached, so just uses a couple nanoseconds of electrons.

[jgabry]

@dpastoor if we put ggplot2 in Imports and add this

ggplot_fns <- readRDS("ggplot_fns.rds")
lapply(ggplot_fns, function(f) {
  assign(f, getFromNamespace(f, "ggplot2"), envir = parent.frame(2))
})

to global_utils.R (where 'ggplot_fns.rds' is a character vector of all function names in ggplot2) then I think everything works. Give this branch a look/try.

I think CRAN would care if this were happening in the R/ directory and/or could possibly affect the global environment, but since this is all in inst/ and only relevant while the app is running it would probably be ok.

[jgabry]

@dpastoor The only thing we'd need to worry about for this approach (I think) would be not to give any function we write and use in the app the same name as one in ggplot2.

[dpastoor]

@jgabry that sounds like a good approach. I wonder (hope not) if we'd also have to do the same for shiny functions. We could probably mock a little bit by calling shinystan::launch_shinystan (without explicitly loading shinystan) and see what breaks

[jrnold]

I'm a little late on this, but would the import functionality of https://github.com/smbache/import help out?

[dpastoor]

I think that import is almost "too elegant" for our purposes. For better or worse this is a hack that gives us the entire flavor of ggplot inside the package, but will also allow single functions to work that depend on ggplot without the explicit attachment that occurs when the shinystan package is loaded (eg the difference between library(shinystan) and shinystan::some_function_depending_on_ggplot.

@jgabry can correct me, but I think the ggplot_fns this is more of a stopgap until we can better define the use cases of shinystan functions outside of shinystan itself, then come up with a better solution (if one is necessary).

[jgabry]

I've never heard of it but I just read the readme and it looks really
useful.

On Wednesday, August 19, 2015, Jeffrey Arnold [email protected]
wrote:

I'm a little late on this, but would the import functionality of
https://github.com/smbache/import help out?

—
Reply to this email directly or view it on GitHub
#85 (comment).

[jgabry]

Thanks for the suggestion. I'll play around with it and see if it can make
this cleaner than the current solution.

On Wednesday, August 19, 2015, Jonah Sol Gabry [email protected] wrote:

I've never heard of it but I just read the readme and it looks really
useful.

On Wednesday, August 19, 2015, Jeffrey Arnold <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> wrote:

I'm a little late on this, but would the import functionality of
https://github.com/smbache/import help out?

—
Reply to this email directly or view it on GitHub
#85 (comment).

[jgabry]

@dpastoor yeah hopefully just a temporary fix. I'm not familiar with the
import package. could it be used online inside the app just to
import ggplot?

On Wednesday, August 19, 2015, Jonah Sol Gabry <[email protected]
javascript:_e(%7B%7D,'cvml','[email protected]');> wrote:

Thanks for the suggestion. I'll play around with it and see if it can make
this cleaner than the current solution.

On Wednesday, August 19, 2015, Jonah Sol Gabry [email protected] wrote:

I've never heard of it but I just read the readme and it looks really
useful.

On Wednesday, August 19, 2015, Jeffrey Arnold [email protected]
wrote:

I'm a little late on this, but would the import functionality of
https://github.com/smbache/import help out?

—
Reply to this email directly or view it on GitHub
#85 (comment)
.

[dpastoor]

@jgabry so from my reading of it it is an attempt to emulate the python import functionality, therefore this is a 'script level' import. The pro is it makes your code more robust against namespace clashes since you can explicitly control for them through both renaming or selectively importing. The downside, is if it is like python (which it seems to be based on reading) means imports occur at the .R file level, at least so it seems. So for example, if we wanted to use this for ggplot, at the top of each file that used ggplot function, we'd have to call import.

I think he alludes to this as noted in the package README

While import will also work for package development, it is meant for R scripts.

I think if we had like a couple ggplot elements that we explicitly wanted to pull in (eg only qplot or the likes) this would be an ideal candidate, however at this point it seems to add an additional dependency and isn't really targeted for, at least at this point, package development