search

rbchan / unmarked

R package for hierarchical models in ecological research
https://rbchan.github.io/unmarked/
36 stars 25 forks source link

Make namespace more explicit, avoid unnecessarily package loads #174

Closed kenkellner closed 4 years ago

kenkellner commented 4 years ago

Moves parallel and Rcpp from depends to imports (so they no longer need to be explicitly loaded when unmarked is), and where possible unmarked now uses importFrom instead of imports. Ideally lattice would also be moved out of depends but doing so causes issues with some vignettes and examples that are expecting it to be loaded.

These changes make unmarked a bit more in line with my understanding of NAMESPACE best practices, but they are not crucial. There are no changes as far as I know to user-facing behavior.

rbchan commented 4 years ago

Looks like there's a conflict that needs to be resolved before this can be merged cleanly. Also, can you double check that it's okay to move Rcpp from Depends to Imports? I think Rcpp might need special treatment. For example, here's what Rcpp::Rcpp.package.skeleton says:

The DESCRIPTION file gains an Imports line requesting that the package depends on Rcpp and a LinkingTo line so that the package finds Rcpp header files.

The NAMESPACE gains a useDynLib directive as well as an importFrom(Rcpp, evalCpp to ensure instantiation of Rcpp.

kenkellner commented 4 years ago

The DESCRIPTION file gains an Imports line requesting that the package depends on Rcpp and a LinkingTo line so that the package finds Rcpp header files.

I took that to mean that Rcpp shows up in the Imports line of DESCRIPTION (confusingly worded by also having depends in the sentence), along with LinkingTo (which is how I set it up in this PR).

This is what Dirk recommends: http://lists.r-forge.r-project.org/pipermail/rcpp-devel/2014-January/006984.html

Also the way dplyr does it: https://cran.r-project.org/web/packages/dplyr/index.html

At any rate, all the tests and checks work with it set up this way.

And sorry, the two PRs both modified the same file so there was a merge issue. I'll clean it up.

rbchan commented 4 years ago

Sounds good.