search

hubverse-org / hubEnsemblesManuscript

https://htmlpreview.github.io/?https://github.com/Infectious-Disease-Modeling-Hubs/hubEnsemblesManuscript/blob/master/analysis/paper/hubEnsembles_manuscript.html
Other
1 stars 2 forks source link

`renv` is sometimes throwing errors #67

Closed lshandross closed 2 months ago

lshandross commented 2 months ago

@harryhoch and I have both been experiencing various errors when trying to run renv::restore(). Based on the current code that's on the main branch, Harry said that the code eventually worked fine but it requires some work to get it running. Even after running renv::restore() and devtools::install() he had to manually install hubEnsembles, hubVis, hubExamples, readr, reshape2, lubridate, patchwork, and cowplot

I had to make install a number of packages manually before I could get it to work. In particular, I had an issue with getting a specific version of arrow to install, which was a dependency for hubData:

FAILED C:\WINDOWS/System32/tar.exe xf "C:\Users\lshandross\AppData\Local/R/cache/R/renv/source/github/arrow/arrow_e03105efc38edca4ca429bf967a17b4d0fbebe40.tar.gz" -C "C:/Users/lshandross/AppData/Local/Temp/Rtmps1IOxY/renv-package-old-23a0c2f7229"

apache-arrow-e03105e/python/cmake_modules: Can't create '\?\C:\Users\lshandross\AppData\Local\Temp\Rtmps1IOxY\renv-package-old-23a0c2f7229\apache-arrow-e03105e\python\cmake_modules': Invalid argument tar.exe: Error exit delayed from previous errors.

Error: error decompressing archive [error code 1]

I was able to get around this by manually installing hubEnsembles, arrow, hubData (though the arrow dependency failed to install again), running arrow::install_arrow(), calling renv::restore(), then finally renv::snapshot.

Since this fixed things for me, I tried pushing these changes to a branch fix-renv and asked Harry to test it. However, he got new errors from running this

devtools::install() Error in loadNamespace(x) : there is no package called ‘devtools’ install.packages(“devtools”) Error: failed to resolve remote ‘Infectious-Disease-Modeling-Hubs/hubEnsembles’ -- error downloading ‘https://api.github.com/repos/Infectious-Disease-Modeling-Hubs/hubEnsembles’ [error code 22]

Hoping for some help from @annakrystalli on how to fix renv so that it works for everyone's machine

annakrystalli commented 2 months ago

So I also had a nightmare with renv:

Anyways I finally managed to trick renv and install the newer ragg version from binary which worked fine and then snapshot it. I've also set renv dependency management to be explicit (i.e. record all dependencies that are specified in the DESCRIPTION file only. Finally I added hubExamples to the DESCRIPTION file as well as the remote to hubData which was missing and updated the README to reflect that. I did all this in the fix-renv branch (changes pushed).

Having said all that, in the meantime I created a new branch remove-renv where I removed all renv infrastructure and used devtools::install(dependencies = TRUE) (to install all dependencies, including suggests) and that worked quickly and painlessly for me. That version of dependency management is available in the remove-renv branch.

I will leave it up to others to decide which method they prefer and merge appropriately. In general I would recommend renv for this type of project, but it's created so many headaches for the team already that I'm not so sure now...

annakrystalli commented 2 months ago

Noting that I managed to reproduce the paper using both methods, both starting with clean libraries (as I've upgraded to R 4.4.0 so many of the dependencies were missing).

sbfnk commented 2 months ago

That reflects my experiences with renv (and even where I had no issues, others often did). See also @bisaloo's nice blog post on the topic https://epiverse-trace.github.io/posts/renv-complications/

If going without renv then I think using devtools::install_dev_deps() would be preferable as we don't need to actually install the paper package (as it's loaded with devtools::load_all() everywhere.

I'd be happy either way but suspect many potential users would have an easier journey with devtools so would lean towards that.

annakrystalli commented 2 months ago

Agreed that load_all() makes the functions available but given folks have gone to the effort of writing roxygen documentation on the functions used in the manuscript, it could be nice for a user to build the package and have those available. Once all the dependencies are installed, it's not much more effort.

Of course, if all the functions are exported, we could just load the library to make them available once installed instead of using load_all().

sbfnk commented 2 months ago

Ah yes that's a good point. So we should change the call in the .qmd file to load the library.

annakrystalli commented 2 months ago

Ah yes that's a good point. So we should change the call in the .qmd file to load the library.

Fixed in cb8590dcbbcd5718b0ac27340a7c6dc895eb52b8 & 3f33120844db8cd8846cb2e6c568180f049f86e1

lshandross commented 2 months ago

Thank you both for your help with this issue! I've also gotten the manuscript to render with both solutions, though I'm personally partial to the solution of removing renv completely given the trouble it's caused us, but I'll let @eahowerton weigh in before we make a final decision.

If we were to go with removing renv, what would be best practice in terms of merging/deleting branches? i.e., should we delete the fix-renv without merging, merge it anyway but then overwrite with the remove-renv branch, or simply leave it as is without deleting the branch but not merging it? @annakrystalli

annakrystalli commented 2 months ago

Sounds like a plan. When you decide which approach you prefer, just merge the relevant branch and delete the other one ☺️

eahowerton commented 2 months ago

I agree, seems best to remove renv. Thank you all for figuring this one out!

lshandross commented 2 months ago

Great, I'll remove it and delete the branch.