extendr / rextendr

An R package that helps scaffolding extendr-enabled packages or compiling Rust code dynamically
https://extendr.github.io/rextendr/
Other
184 stars 27 forks source link

Remove usethis dependency #70

Closed clauswilke closed 3 years ago

clauswilke commented 3 years ago

We currently only use usethis for usethis::write_over(). I think we can remove this dependency, in particular if we take the position that we don't write any files if src already exists.

Ilia-Kosenkov commented 3 years ago

I would like to initiate a discussion here -- before drawing any conclusions. Currently {rextendr} provides the following functionality:

  1. Dynamically compiling Rust source, dynamically loading libraries, and invoking native methods. Rust source typically comes in a character (string) form. This is useful for simple prototyping and Rust exploration directly from R interactive session.
  2. Package documentation & compilation. This is mainly achieved through rextendr::document(), which compiles Rust code, generates wrappers, and then invokes roxygen2::roxygenize() to parse documenting tags and generate exports & docs. This is used for package development.
  3. Package scaffolding/environment setup. This is achieved using rextendr::use_extendr(), which prepares a package for the integration with extendr. This step is awfully similar to what usethis::use_* functions do.

There is #60, which suggests introducing use_extendr_github_action(), a functionality similar to usethis::use_github_action(). To me, it looks like these two functions (rextendr::use_extendr(), rextendr::use_extendr_github_action()) should follow {uesthis} conventions as closely as possible, which will make the workflow easy for package developers familiar with {usethis}. {usethis} has a notion of projects, global configurations that help with path resolution (if a project is not configured, then {rprojroot} is used to find package root), and we can also benefit from this. If you inspect rextendr::use_extendr, you can see that it does not rely on anything {rextendr}- or extendr-related, it just writes a bunch of files. This function can be safely moved (in the future, if we ever will be able to do this) to {uesthis} itself, at which point it will have to use {usethis} infrastructure to be in line with other {usethis} methods. This is another argument in favor of not only not removing {usehtis} dependency, but actually embracing it, at least in rextendr::use_* methods.

In this bright future I mention, we could rewrite rextendr::use_extendr() as

# Omitting function arguments
use_extendr <- function(...) {
  usethis::use_extendr(...)
}

Possibly adding {lifecycle} soft-deprecation warning.

This topic is important (at least to me) because #60 cannot be implemented unless a decision is made whether we embrace {usethis} and aim at integration with it or not. If we do want a close integration, we can directly adopt (or at least draw inspiration from) usethis::use_github_action(), as it is a MIT-licensed package.

clauswilke commented 3 years ago

In this bright future I mention, we could rewrite rextendr::use_extendr() as ...

In my opinion, it would make more sense the other way round, that usethis calls rextendr, because the extendr ecosystem is going to be young and somewhat in flux for several years and usethis may not want to commit to having to maintain the default template files for us.

But either way, I'm not going to die on this hill. If adding usethis solves a meaningful problem that would be difficult to solve otherwise I won't be in the way of adding this dependency.

Ilia-Kosenkov commented 3 years ago

Ok, sounds fair. Then I suggest to at least implement something like

write_lines <- function(lines, file, quiet = FALSE) { ... }

to avoid boilerplate user notification code, somewhat similar to what usethis::write_over() does.

clauswilke commented 3 years ago

Sure. Could you prepare a PR?

Ilia-Kosenkov commented 3 years ago

Yeah, I will do that.