rgiordan / zaminfluence

Tools in R for computing and using Z-estimator approximate influence functions.
Apache License 2.0
94 stars 10 forks source link

package gridExtra required but missing from depends? #42

Closed michaelpollmann closed 3 weeks ago

michaelpollmann commented 2 years ago

I just tried to install the zamfluence package on a fresh R installation:

devtools::install_github("https://github.com/rgiordan/zaminfluence/",
                         ref="master",
                         subdir="zaminfluence",
                         force=TRUE)

and got the error message

* installing *source* package 'zaminfluence' ...
** using staged installation
** R
** inst
** byte-compile and prepare package for lazy loading
Error in library(gridExtra) : there is no package called 'gridExtra'
Error: unable to load R code in package 'zaminfluence'
Execution halted
ERROR: lazy loading failed for package 'zaminfluence'

In the description, you have https://github.com/rgiordan/zaminfluence/blob/2d29fbbf5f2b69a26ad391ff4e79d117ee152fbd/zaminfluence/DESCRIPTION#L11 and only "suggest" gridExtra -- that appears to not be sufficient here? https://github.com/rgiordan/zaminfluence/blob/2d29fbbf5f2b69a26ad391ff4e79d117ee152fbd/zaminfluence/DESCRIPTION#L17-L18

Installation was successful for me after I manually installed gridExtra

I've limited experience with making packages, and didn't search the code for how you are using gridExtra. When I made a package myself, I used "Imports" -- e.g. https://github.com/michaelpollmann/parTreat/blob/4bad92f8307533b5c56d2e4e25c5a628db83ea66/DESCRIPTION#L15-L17 and then in the code always do e.g. reticulate::<function name>, which avoids adding those packages to the namespace of the user (after library(zamfluence) also all functions from the package listed in depends are available to the user)

When first loading the package, it also had to download something from pytorch.org and googleapis.org -- not sure if one can make that happen during package installation (it may be coming form the torch package)

The actual package works well for me so far -- thanks for sharing the code for your paper!

rgiordan commented 3 weeks ago

Wow, I should watch my issues more. Thanks for catching this and the detailed analysis. Fixed in https://github.com/rgiordan/zaminfluence/pull/46