Skip to content
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

depends vs imports on ggplot/shiny #83

Closed
dpastoor opened this issue Aug 12, 2015 · 3 comments
Closed

depends vs imports on ggplot/shiny #83

dpastoor opened this issue Aug 12, 2015 · 3 comments

Comments

@dpastoor
Copy link
Collaborator

In regards to #82 it might be worth discussing the pros/cons of depends/imports.

With the depends, since the package requires loading on startup, you can 'get away' with not explicitly calling functions referencing the ggplot2 namespace. This makes it faster/easier to code inside the package, but results in situations like the above issue.

Also, anecdotally, I have run into some funky behavior with packages that force attaching as if you have similarly named functions your expectation of where a function sits in the namespace path can get screwy.

For example using the classic plyr/dplyr issue of using same verb names:

given a library poor_dependencies the depends on plyr,

library(dplyr)
library(poor_dependencies)

will result in plyr being attached second thus overwriting dplyr, which was hell to track down the first time this occurred.

Shiny and ggplot are reasonably well-behaved and have explicit-enough naming conventions that it is usually pretty easy to stay away from namespace clashes, so for your package I don't think those concerns are too big of a deal, and given shinystan, there is essential functionality in ggplot, so even by hadley's suggestions it is reasonable to use depends in this situation.

I guess the question is do we want to track down all the ggplot elements in all the source code :-o

if you want to move that direction, let me know and I'll start tracking things down in the update_ggplot_elements branch

@jgabry
Copy link
Member

jgabry commented Aug 12, 2015

I agree with all of this, and I'm not sure what the right answer is for ggplot2. As you say, shinystan really does depend on ggplot2 and my hunch is that any problems with depending on ggplot2 and not using ggplot2::fun would be limited to cases like the one in #82 and wouldn't occur in regular use of shinystan outside other packages.

However, it's cool that someone's building a package that uses shinystan and it'd be nice to support such endeavors. The easiest/quickest solution would be to just say that any package that is going to call launch_shinystan should also depend on ggplot2, as I don't think ggplot2 would interfere with much.

But I'm not opposed to going the ggplot2::fun route. Let me take a look at a few things quickly and then I'll post a follow-up.

@dpastoor
Copy link
Collaborator Author

sure for now I'm tracking in #84 it looks like it shouldn't be that hard to nail down most things by just recursively searching for specific components in ggplot

@jgabry
Copy link
Member

jgabry commented Aug 12, 2015

It should also be almost entirely within the files under helper_functions, with the exception of the ppcheck ones, which aren't in the same directory for a reason that isn't relevant anymore.

jgabry added a commit that referenced this issue Aug 19, 2015
Moves ggplot2 to Imports from Depends. Resolves #85, resolves #83, and resolves #82
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

No branches or pull requests

2 participants