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

make Lahman and hflights suggested instead of required, for Packrat #508

Closed
HarlanH opened this issue Jul 25, 2014 · 8 comments
Closed

make Lahman and hflights suggested instead of required, for Packrat #508

HarlanH opened this issue Jul 25, 2014 · 8 comments
Assignees
Labels
Milestone

Comments

@HarlanH
Copy link

@HarlanH HarlanH commented Jul 25, 2014

Currently, when installing dplyr in Packrat projects, these two data dependencies are included too, superfluously. There's no easy workaround in Packrat, but changing their status here would mitigate the problem.

@hadley
Copy link
Member

@hadley hadley commented Jul 28, 2014

Hmmm, I didn't think of that. I originally moved Lahmans and hflights to required so that the examples would all work without many if(require()) tests.

@kevinushey any thoughts?

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Jul 28, 2014

IMO -- they're data packages and they belong in Suggests, and the if (require(...)) cruft is a necessary evil. Or perhaps that can be abstracted / hidden in some way...

Although, packrat can remedy this a bit with the global cache (so there's only one copy of the data packages for all projects).

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Jul 28, 2014

Can this be a roxygen tag or something expressing that these examples need these additional packages.

@hadley
Copy link
Member

@hadley hadley commented Jul 28, 2014

@romainfrancois I don't think so - you need some condition in the example to avoid the code being run when the dependency isn't available

@romainfrancois
Copy link
Member

@romainfrancois romainfrancois commented Jul 28, 2014

well, bare with me as I don't know how roxygen is written, but this could be an annotation that:

  • populates Suggests
  • wraps the whole example in if(require(.)) { ... }

but perhaps there are cases where some part of the example uses package x and some other part uses package y ... anyway, just an idea.

@hadley
Copy link
Member

@hadley hadley commented Jul 28, 2014

Yeah, it starts to get pretty complicated. Plus having tags that modify other tags means that you need to process them in a certain order, which I've been trying to avoid.

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Jul 30, 2014

I'm beefing up the logic in packrat to allow for easier use of 'external' packages across projects, in a way that's compatible with R's regular package management.

After I've worked things out, the idea is that someone can initialize a packrat project, and add these data packages as external packages. For example, if I initialize a packrat project with:

packrat::init(options = list(external.packages = c("BH", "Rcpp", "Lahman", "hflights")))

and then install dplyr with install.packages("dplyr"), only dplyr, assertthat, and magrittr get downloaded and installed into the private library -- with the other packages being 'used' from the user library.

This breaks the packrat isolation + reproducibility a bit, but is convenient enough that it's sensible for packages which are relatively dependency-free and stable.

@hadley hadley added this to the 0.3 milestone Aug 1, 2014
@hadley hadley self-assigned this Aug 1, 2014
@hadley hadley closed this in 1d8b2e2 Aug 27, 2014
@hadley
Copy link
Member

@hadley hadley commented Aug 27, 2014

I just bit the bullet on this, and it turned out to not be that bad. Enjoy :)

krlmlr pushed a commit to krlmlr/dplyr that referenced this issue Mar 2, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jun 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants