Feature suggestions for rstan_package_skeleton

[stan-dev]

Feature suggestions for rstan_package_skeleton

[jgabry]

Thanks for doing all this work! I haven’t had a chance to dig into it yet, but Ben and I will definitely go through this and get back to you soon (might be more than a few days realistically but probably not too much more). What I can say now is that, all else equal, we are definitely in favor of efforts to reduce the variation in the build processes across the Stan ecosystem in R.

…

On Sat, Apr 21, 2018 at 7:30 PM mlysy ***@***.***> wrote: Dear @jgabry <https://github.com/jgabry> and @bgoodri <https://github.com/bgoodri>, Here is a link <https://github.com/mlysy/rstantools> to an *rstantools* fork providing a few additional features to the current version of rstan_package_skeleton, namely: 1. The ability to add your own C++ code to a Stan-enabled package. 2. The ability to add Stan functionality to an existing *R* package. I've extensively documented my work in the repo's README <https://github.com/mlysy/rstantools>. Aside from these features, I follow the current build mechanism of rstan_package_skeleton very closely. There are however a few notable exceptions: 1. I use package.skeleton as the driver rather than usethis::create_package. This is mainly because I was following the previous version of *rstantools* when I created the fork. 2. In my version, every time a .stan file get added/deleted/modified requires slight manual intervention, i.e., a call to rstantools::rstan_config(). The current version of *rstantools* circumvents this through the Makevars. However, for reasons outlined in the README <https://github.com/mlysy/rstantools>, this Makevars will generate a WARNING from R CMD check. I'm in the process of adapting my fork to circumvent this additional call to rstan_config. With your permission, would love to turn this into a PR -- either way, looking forward to hearing your thoughts! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#30>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHb4Q8SZWeLRPNHEfXamX8k0pN6yQpDZks5tq8EVgaJpZM4TeqLE> .

[jgabry]

@mlysy sorry for the delay in getting back to you. Yes, a pull request would be great! Would it be possible to make a PR that doesn't break the 1.5.0 release? That is, can you submit a PR that adds what you're proposing to the changes we made in the latest release so we don't have to, for example, immediately backtrack to using package.skeleton(), etc? If that's possible then we would more than welcome a pull request with the vast majority of what you're proposing! Thanks for all the work you've already done on this.

[jgabry]

One question just to clarify: was your proposal to replace the current rstan_package_skeleton with the contents of rstan_package_skeleton_plus or to have the _plus version as a separate function. I assume the former, right?

[mlysy]

@jgabry delighted that you are OK with me submitting a PR! 😆

• Yes, I did intend for rstan_package_skeleton to be overwritten with the current contents of _plus.

• As for replacing the internal call to utils::package.skeleton by one to usethis::create_package, I am more than happy to do so! One thing though: can I suggest that we stick more closely with the original function signature, namely:

  rstan_package_skeleton <- function(name, path, code_files, stan_files, ...)

  I probably skipped a few args, but the reason I ask is that many people have written package.skeletons and they all require more or less the same inputs. A few examples are found in packages Rcpp, RcppEigen, RcppArmadillo, Rclusterpp, and SimInf. So I and perhaps other useRs expect these functions to work a certain way.

  On the other hand, functions with a similar purpose to package.skeleton but with very different signatures tend to be named differently. Some examples I'm aware of are usethis::create_package, devtools::create and skeletor::skeletor.

  Hope I'm not being too nit-picky here...let me know what you think!

[jgabry]

Great! I actually think the best option might be to offer rstan_package_skeleton (with arg names changed back like you say) and also create_rstan_package, which is the same function internally (probably one a wrapper of the other) but with a different signature. That way we cater to both crowds. What do you think?

[jgabry]

I also definitely think we should have the use_rstan function you added

[jgabry]

General comment: given that rstan_package_skeleton is only used to create packages and not for calling within a package we have a bit more flexibility with making breaking changes if they are substantial improvements.

[mlysy]

I love it! I need to spend the next few days on my day-job but will have the PR ready shortly.

[jgabry]

Awesome. No rush!

[jgabry]

closing this now that #44 is finally merged!