usethis icon indicating copy to clipboard operation
usethis copied to clipboard

[feature request] Implement `use_compat()` for managing common `compat-[pkg].R` files

Open wurli opened this issue 1 year ago • 14 comments

Often when writing a new package I find myself simply lifting scripts like compat-purrr.R from the {rlang} source-code to avoid taking on more dependencies than necessary. I think formalising this process would be well worth the effort. E.g. it could be done as follows:

  • use_compat(pkg) would copy a compat-pkg.R into the user's R directory, e.g. from the github for an existing package such as {rlang} where the 'master' version is defined, or from a dedicated repo for tidyverse compat- files. The second option seems better to me as it would also provide a nice place to look for such files without necessarily having to go through {usethis}.
  • Re-running use_compat() after the 'master' version has changed could update the existing compat- script after prompting the user to confirm any (potentially breaking) changes.
  • use_compat() could also support single functions, e.g. in the case of withr::defer(), which has its own compat-defer.R file in the {rlang} source code.
  • Specifically, I think standardising 'compat' files for at least the following packages would be really useful: {cli}, {dplyr}, {forcats}, {glue}, {purrr}, {rlang}, {stringr}, {vctrs}, {withr} and {zeallot}

Obviously the real value here would be in having such compat- scripts readily available. However it sees like this is already somewhat of a 'soft-supported' service from the Tidyverse, to wit, rlang/compat-vctrs.R begins with the following line:

# Latest version: https://github.com/r-lib/rlang/blob/main/R/compat-vctrs.R

So, perhaps the work is already largely done and the creation of use_compat() would mainly be a case of compiling all compat- files which already exist.

wurli avatar Jul 25 '22 12:07 wurli

This suggestion feels along the same lines as use_github_action(), where we help folks pull copies of workflow config files from r-lib/actions.

Do you have any thoughts on this @lionel-?

jennybc avatar Aug 29 '22 22:08 jennybc

See also https://github.com/r-lib/rlang/pull/1401

I do think it would be useful. We didn't add it to rlang immediately because we thought we might need some kind of dependency system (i.e. compat-foo.R might need compat-bar.R to work, which does occur in some rlang compat files). But maybe we can do without that for now, and just document the dependencies like this https://github.com/r-lib/rlang/blob/b45acd370617c307e72df8a60e7ef35ad1b0c05a/R/compat-types-check.R#L6

DavisVaughan avatar Aug 29 '22 22:08 DavisVaughan

Ah, I didn't realize this was already under way in rlang.

jennybc avatar Aug 29 '22 22:08 jennybc

yup I've been meaning to take a look at implementing a simple dependency and version system for the rlang compat files. Also it would be nice to record all the compat files used by the package in a file so that you could do usethis::update_compat().

A few years ago @hadley thought this wouldn't be worth the work, but since we have now more compat files than we used to, I think this might now be worth it. I especially like that compat files allow us to refine experimental APIs without causing revdep breakages. They are a good playing field for emerging ideas and some more automatic tooling would be helpful.

lionel- avatar Aug 30 '22 09:08 lionel-

Thanks for the comments, I also wasn't aware of the discussion underway in rlang. Regarding dependencies, I really like the idea of documenting dependencies on other compat- files, but this might get tricky. E.g. consider the following:

  • My package {foo} imports {rlang}
  • {foo} also uses a compat-purrr.R script
  • compat-purrr.R has a dependency on compat-rlang.R, but the {rlang} import makes this redundant whilst also potentially changing behaviour in some edge cases.

Options of dealing with this are:

  1. Develop a more sophisticated system for managing dependencies in compat- files (perhaps the simplest implementation is to use {pkg} if available and compat-pkg.R if not?)
  2. Make all compat- files dependency-free

To be honest I would lean towards option 2, since building up a sophisticated dependency system for compat- files seems to defeat the point of having them in the first place.

wurli avatar Aug 30 '22 09:08 wurli

compat-purrr.R has a dependency on compat-rlang.R, but the {rlang} import makes this redundant whilst also potentially changing behaviour in some edge cases.

Just to be clear compat-purrr.R uses rlang directly, it doesn't depend on compat-rlang.R. But other files use compat-rlang.R so that they only use rlang conditionally, if installed. Currently the functionality in compat-rlang is inlined in those files.

We also have compat-types-check.R which depends on compat-obj-type.R. In this case the functionality is not inlined and you have to manage the dependency yourself.

I would like something (a) more consistent and (b) with less manual work. What I don't want is a "sophisticated" dependency system, we should only consider a simple one.

lionel- avatar Aug 30 '22 09:08 lionel-

Just to be clear compat-purrr.R uses rlang directly, it doesn't depend on compat-rlang.R. But other files use compat-rlang.R so that they only use rlang conditionally, if installed. Currently the functionality in compat-rlang is inlined in those files.

Ah - thanks for clarifying 👍

I would like something (a) more consistent and (b) with less manual work. What I don't want is a "sophisticated" dependency system, we should only consider a simple one.

Do you think these requirements would be met by limiting the scope of use_compat()? E.g. by assuming each each compat- file will cover single package as opposed to a feature/function?

Apologies if I'm a step behind here - I'm not sure if compat-types-check.R is providing compatibility with another package or not.

wurli avatar Aug 30 '22 09:08 wurli

it would be nice to record all the compat files used by the package in a file

Random thought that we might could do:

Config/usethis/compat:
    r-lib/rlang/purrr,
    r-lib/rlang/types-check

Which usethis::use_compat() would automatically parse and pull in

DavisVaughan avatar Aug 30 '22 12:08 DavisVaughan

Interesting idea.

lionel- avatar Aug 30 '22 13:08 lionel-

So we're all aware while thinking about this, two pre-existing packages deal with a similar problem:

  • https://github.com/hrbrmstr/freebase
  • https://github.com/yonicd/bplyr

I haven't seen much about them since they first came out, though, so I don't think they much took off. One of the authors here seems to be frustrated that tools he's developed are ignored without discussion, so I thought it best to bring it up.

malcolmbarrett avatar Aug 30 '22 14:08 malcolmbarrett

I think the packages above are solving a different problem. Maybe I'd call them "compat packages"?

I think the problem we are trying to solve is how we vendor .R files from one package to another. This has some of the same motivations as "compat packages" (lack of dependencies), but I don't see the overlap between what we're discussing here and the actual packages above.

jennybc avatar Aug 30 '22 15:08 jennybc

freebase is like that, actually. It stores files that get copied into a package rather than being a dependency itself. In other words, it deals with single compat files, as well, and in fact, uses usethis-style code to do so. bplyr is what you call a compat package, though.

malcolmbarrett avatar Aug 30 '22 15:08 malcolmbarrett

Vendoring an entire package is something that @gaborcsardi does often and has tools for, IIRC. But I agree with Jenny that this is a different problem than single compat files.

lionel- avatar Aug 30 '22 15:08 lionel-

Another solution in this space is staticimports to statically import functions from another package. It uses a roxygen2-style import syntax and imports individual functions with automatic function-dependency resolution. It requires exported functions be placed in inst/staticexports though, but I think the approach could be extended to package code.

What's nice about staticiports is that, in theory, you could import check_bool() from rlang without worrying about where it comes from or its dependencies — staticimports will automatically inline stop_input_type() and check_bool()'s other function deps without you having to copy both compat-types-check.R and compat-obj-type.R.

gadenbuie avatar Aug 30 '22 15:08 gadenbuie

If we're going to make this work, I think we're going to need the compat files to have more machine readable structure. I'd suggest we mimic the top of a .Rmd, like so:

# ---
# src-repo: r-lib/rlang 
# src-file: compat-types-check.R
# dependencies: [compat-obj-type.R]
# last-updated: 2022-08-11
# ---
#
# ## Changelog
# 
# 2022-08-11:
# - Added changelog.
# 
# nocov start 

...

# nocov end

And then I think we need some additional text at the top of the copied files to make it clear that they come from another repo, maybe just something like:

# ---------------------------------------------------------------------------------
# Compatiblity file inlined from rlang/R/compat-types-check.R: do not edit by hand
# ---------------------------------------------------------------------------------

Maybe we should also give the created compat file a different prefix?

hadley avatar Jan 18 '23 14:01 hadley

Actually maybe the src file doesn't need src-repo and src-file since they can be automatically added.

hadley avatar Jan 18 '23 14:01 hadley

Or maybe just a src like https://github.com/r-lib/rlang/blob/main/R/compat-cli.R.

hadley avatar Jan 18 '23 14:01 hadley

Just re-read this for first time in a while. If you work on it @Hadley, use_github_file() is a very handy function for this sort of thing.

jennybc avatar Jan 18 '23 17:01 jennybc

Naming suggestion from @lionel-:

standalone-purrr.R        # Source
import-standalone-purrr.R # Target

hadley avatar Jan 30 '23 17:01 hadley