subugoe / hoad

Deprecated: Please check https://github.com/subugoe/hoaddash
https://github.com/subugoe/hoaddash
GNU Affero General Public License v3.0
15 stars 4 forks source link

assert necessary deps for `runHOAD()` with good error msg #202

Open jhoeffler opened 4 years ago

jhoeffler commented 4 years ago

flexdashboard writexl viridis

maxheld83 commented 4 years ago

These are, in fact, all listed in DESCRIPTION under Suggests:.

I am guessing they were not installed on your local machine, because you ran remotes::install_github() or some such thing with the default settings. If you run remotes::install_github(..., dependencies = TRUE) then everything listed in Suggests: will also be installed.

Suggests: in DESCRIPTIONs are canonically used for "additional" dependencies not needed to run the core functionality of a package (as covered by R CMD check), but other things like vignettes or, in this case, a shiny app shipped with the package. This distinction is helpful to reduce the absolutely necessary dependencies for the core functionality as much as possible.

The package is already way too fat in terms of dependencies as per #179 and we'll have to further slim this list down.

You can also avoid all of this installation trouble by using the docker image for {hoad} which should come with all the necessary dependencies already installed as documented in https://subugoe.github.io/hoad/CONTRIBUTING.html.

Hope this helps.

maxheld83 commented 4 years ago

as discussed off-gh with @jhoeffler

maxheld83 commented 4 years ago

I think the most idiomatic way to do this would be to add an assertion inside runHOAD() to make sure that the deps are all available. So this wouldn't be documentation, but just to give an informative error message when things don't work (i.e. launching the dashboard).

I think it's generally not expected that users are able to reproduce vignettes and other ancillary components with the default dependencies = NA.

maxheld83 commented 4 years ago

just to add one more thing to the off-gh discussion @jhoeffler; running remotes::install_github(..., dependencies = TRUE) also wouldn't be exactly what you'd need to run the dashboard. It might actually very well install too many dependencies, including some that are only required at build time or those required to build the vignettes.

Instead, we should add a precise assertion to runHOAD() which recommends users install only the missing packages necessary for the shiny app, not all Suggests: dependencies, which may be way to many.

This may be a bit nitpicky 😼 but I think in general it pays to be as stingy as possible with dependencies, and it also pays to be as explicit and narrow as possible in the intent (here: the dependencies necessary for the app, not all Suggests:). (Don't think the last sentence was proper english, but whatever)