rlemon - import or suggests

[benthestatistician]

Could the rlemon dependency be declared in Suggests rather than Imports, just as the relaxiv dependency is? If so, it would be helpful in situations where one can get relax-iv but not rlemon to build (cf rlemon#43). In R/* I find exactly one token each of rlemon and relaxiv (excluding instances of just the strings), at fmatch.R#L225 and fmatch.R#243, respectively. This suggested to me that maybe we could get away with giving them the same treatment within DESCRIPTION.

[benthestatistician]

@josherrickson @nullsatz

[josherrickson]

I believe Suggests are not automatically installed, which could leave users installing a package that won't work. We could add a graceful error message informing users, but that adds quite a bit of friction to require users to install a second package. To me, the slight gain here doesn't seem worth it, but I'm willing to be convinced otherwise.

[benthestatistician]

I'll argue that that's worth the tradeoff. Although matching is our core function, we also provide utilities for looking at matches and so forth. If you can't install the package then you can't inspect an optmatch-created object that may have been generated previously. That's the situation I found myself in:

> foo = readRDS("foo.rds")
> is(foo)
Loading required package: optmatch
Error in .requirePackage(package) : 
  unable to find required package ‘optmatch’
In addition: Warning message:
In library(package, lib.loc = lib.loc, character.only = TRUE, logical.return = TRUE,  :
  there is no package called ‘optmatch’

This foo turned out to be an ISM, but I couldn't figure this out without moving the data to another machine. At the moment, a user who can't get either solver package to install is without options in scenarios like this.

[josherrickson]

This seems more like an argument for spinning ISM/BISM/optmatch into their own package? Take the devtools approach and split the overall optmatch package into smaller packages?

[benthestatistician]

(methinks we already did that, in separating out the optimizers from all else)

[josherrickson]

Well, sure, we did a bit, but I mean take it further. Specifically in this case moving the two primary classes (ISM and optmatch) into packages that users could potentially load without needing any solver, so they can view our output.

[josherrickson]

I still think this isn't a good idea, but I found an example of a package that does something like this: covr

It has DT and htmltools in Suggests: https://github.com/r-lib/covr/blob/40122df12bc9ef1e577dd0720a895b5340b1516f/DESCRIPTION#L57-L64 but will error upon calling report() (which loads the actual report): https://github.com/r-lib/covr/blob/40122df12bc9ef1e577dd0720a895b5340b1516f/R/report.R#L24-L26

We could do something similar. Move rlemon to Suggests next to rrelaxiv, then in match_on or fmatch (or something else), error that "At least one of rlemon or rrelaxiv must be installed." I do wonder if perhaps we didn't even mention rrelaxiv in the error, given that most users, especially those that would install our package fresh without one of the solvers, wouldn't need to understand the difference.

[benthestatistician]

A check/error message as you describe would fit nicely into handleSolver().

As you might expect, I still like the concept, even more so with knowledge of this important precedent in covr. But not so much that I feel the need to force it on my co-developers. Let's wait until we've had the chance to discuss before moving forward.