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

Feature request: use_rcpp_armadillo() and use_rcpp_eigen() #421

Closed
alexpghayes opened this issue Aug 1, 2018 · 4 comments
Closed

Feature request: use_rcpp_armadillo() and use_rcpp_eigen() #421

alexpghayes opened this issue Aug 1, 2018 · 4 comments
Labels
code 🖥️ feature wip

Comments

@alexpghayes
Copy link

@alexpghayes alexpghayes commented Aug 1, 2018

RStudio provides package templates for packages that use Rcpp and RcppArmadillo, but if you decide to use RcppArmadillo mid-project it can be a bit tricky to remember what goes where. I use RcppArmadillo infrequently and can never remember the setup that needs to happen. Having helpers

  • use_rcpp_armadillo
  • use_rcpp_eigen

would be incredibly convenient.

duckmayr added a commit to duckmayr/usethis that referenced this issue Sep 26, 2018
usethis already has use_rcpp() to set up packages to use Rcpp.
Similarly, it would be helpful to have use_rcpp_armadillo() and
use_rcpp_eigen() to set up packages to use RcppArmadillo and RcppEigen
(@alexpghayes requested this feature in issue r-lib#421).
This commit adds both of these functions, as well as tests for them.

use_rcpp_armadillo() ensures Rcpp and RcppArmadillo are in "LinkingTo"
in the DESCRIPTION and that Rcpp is in "Imports", creates the src/
directory and an appropriate .gitignore, as use_rcpp() does,
sets up the proper src/Makevars and src/Makevars.win files (respecting
any existing content in those files), informs the user how to fix up
their NAMESPACE file, and instructs them to run devtools::document().

Similarly, use_rcpp_eigen() ensures Rcpp and RcppEigen are in
"LinkingTo" and "Imports" in the DESCRIPTION, creates the src/
directory and an appropriate .gitignore, as use_rcpp() does,
informs the user how to fix up their NAMESPACE file, and instructs
them to run devtools::document().
duckmayr added a commit to duckmayr/usethis that referenced this issue Sep 26, 2018
usethis already has use_rcpp() to set up packages to use Rcpp.
Similarly, it would be helpful to have use_rcpp_armadillo() and
use_rcpp_eigen() to set up packages to use RcppArmadillo and RcppEigen
(@alexpghayes requested this feature in issue r-lib#421).
This commit adds both of these functions, as well as tests for them.

use_rcpp_armadillo() ensures Rcpp and RcppArmadillo are in "LinkingTo"
in the DESCRIPTION and that Rcpp is in "Imports", creates the src/
directory and an appropriate .gitignore, as use_rcpp() does,
sets up the proper src/Makevars and src/Makevars.win files (respecting
any existing content in those files), informs the user how to fix up
their NAMESPACE file, and instructs them to run devtools::document().

Similarly, use_rcpp_eigen() ensures Rcpp and RcppEigen are in
"LinkingTo" and "Imports" in the DESCRIPTION, creates the src/
directory and an appropriate .gitignore, as use_rcpp() does,
informs the user how to fix up their NAMESPACE file, and instructs
them to run devtools::document().
@duckmayr
Copy link

@duckmayr duckmayr commented Sep 26, 2018

I came to open an issue on this also. I went ahead and wrote up these functions (and tests for them), and I'm ready to open a pull request as soon as "someone from the team agrees" (the prerequisite given in .github/CONTRIBUTING.md).

@jennybc
Copy link
Member

@jennybc jennybc commented Sep 26, 2018

@duckmayr usethis is "resting" right now but you should go ahead and make the PR.

@duckmayr
Copy link

@duckmayr duckmayr commented Sep 27, 2018

@jennybc Done, thanks! I think the commit message was clear and complete, but feel free to reach out with questions. When you come back to it, something to note is it looks like there was a snag on the AppVeyor build when I opened the PR: The tests I added for the new functions failed as the errors "'RcppArmadillo' must be installed before you can take a dependency on it." and "'RcppEigen' must be installed before you can take a dependency on it." were thrown from use_dependency(), which those functions call. R CMD check went through fine on my machine as I have those packages installed. Since those packages won't be dependencies of usethis, I'm not sure what the best practice is to avoid having that test fail on machines that don't have RcppArmadillo and RcppEigen installed -- have it just be a manual test? Have a check before the tests and only run them if requireNamespace("RcppArmadillo", quietly = TRUE) or requireNamespace("RcppEigen", quietly = TRUE) respectively return TRUE?

@jennybc
Copy link
Member

@jennybc jennybc commented Sep 27, 2018

Yeah have a look around the tests for other ways to handle, such as:

  • skip_if_not_installed()
  • use a local mock for use_dependency() to always return TRUE

@hadley hadley added feature wip code 🖥️ labels Nov 24, 2018
jennybc pushed a commit that referenced this issue Mar 28, 2019
* Add use_rcpp_armadillo() and use_rcpp_eigen(). (closes #421).

Note: This is the second coming of PR #601 based on feedback from @jennybc. She requested the design pattern in #645 for testing a package without needing to add it to the Suggests field. This is important as RcppArmadillo has a hard dependency on R >= 3.2.

* usethis itself doesn't need RcppEigen in Suggests

* Style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code 🖥️ feature wip
Projects
None yet
Development

No branches or pull requests

4 participants