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

replace package finding logic with packrat #58

Open
AdamSpannbauer opened this issue Apr 2, 2019 · 4 comments
Open

replace package finding logic with packrat #58

AdamSpannbauer opened this issue Apr 2, 2019 · 4 comments
Assignees
Labels
enhancement New feature or request on-hold Waiting on some externality to happen before working on.

Comments

@AdamSpannbauer
Copy link
Collaborator

yeah. it does it better

@bmewing bmewing added this to the initial-cran-release milestone Apr 2, 2019
@bmewing bmewing added the enhancement New feature or request label Apr 2, 2019
@AdamSpannbauer
Copy link
Collaborator Author

I added a first pass and it fails devtools::check from a CRAN perspective. Using non-exported functions is kind of a no-go from a safe code perspective (i.e. packrat could change what the non-exported function is doing without warning since they don't intend for others to use it).

We'd have to gut the logic and make it our own.

@AdamSpannbauer
Copy link
Collaborator Author

from R packages by Hadley

Similarly you are not allowed to use ::: to access non-exported functions from other packages. Either ask the package maintainer to export the function you need, or write your own version of it using exported functions. Alternatively, if the licenses are compatible you can copy and paste the exported function into your own package. If you do this, remember to update Authors@R.

I think if we go with packrat's process, then it's worth posting an issue in packrat requesting the functionality we're after to be exported. The entirety of the logic they use would be a lot to copy/paste/maintain. It supports pulling dependencies from .R, .Rmd, .Rnw, & .Rpres. I pulled all the logic just for .R, and it would be maintainable IMO, but the logic for the inlcusion of all the formats gets fairly large/complex.

The current logic 'supports' all these file types given that it uses dumb string matching logic rather than parsing expressions. The dumbness grants flexibility at the cost of 'safety' (i.e. listing a string with ':::' being a dependency).

@AdamSpannbauer
Copy link
Collaborator Author

I'd say we put this issue on hold to see what packrat devs respond to my issue with. If they refuse we should discuss how to best proceed.

@AdamSpannbauer AdamSpannbauer added the on-hold Waiting on some externality to happen before working on. label Apr 5, 2019
@AdamSpannbauer
Copy link
Collaborator Author

Got a response on the issue in packrat. Working is being done on renv to replace packrat. renv will have renv::dependencies() that could serve us well. Not sure release date for renv, but it is currently not on CRAN

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request on-hold Waiting on some externality to happen before working on.
Projects
None yet
Development

No branches or pull requests

2 participants