make vol2birdR suggested

[bart1]

This should resolve #557

[bart1]

It seems there are locally still some error but they seem unrelated to this push

[codecov-commenter]

Codecov Report

Merging #565 (58c4d61) into master (57085a3) will decrease coverage by 0.09%.
The diff coverage is 56.00%.

❗ Current head 58c4d61 differs from pull request most recent head 1c354c9. Consider uploading reports for the commit 1c354c9 to get more accurate results

@@            Coverage Diff             @@
##           master     #565      +/-   ##
==========================================
- Coverage   67.89%   67.80%   -0.09%     
==========================================
  Files          58       58              
  Lines        3152     3165      +13     
==========================================
+ Hits         2140     2146       +6     
- Misses       1012     1019       +7     

Impacted Files	Coverage Δ
R/apply_mistnet.R	0.00% <0.00%> (ø)
R/bioRad-deprecated.R	0.00% <0.00%> (ø)
R/nexrad_odim.R	0.00% <0.00%> (ø)
R/calculate_vp.R	90.07% <50.00%> (-2.58%)	⬇️
R/read_pvolfile.R	74.86% <60.00%> (-0.14%)	⬇️
R/zzz.R	60.00% <66.66%> (+10.00%)	⬆️
R/download_pvolfiles.R	93.44% <100.00%> (+0.10%)	⬆️
R/hooks.R	100.00% <100.00%> (+33.33%)	⬆️

[peterdesmet]

I would like @adokter and/or @iskandari to review this as well.

[adokter]

current behavior expects the package to be on CRAN. As long that is not the case, we should have a more informative message, and preferably trigger a devtools::github_install action

[bart1]

Remarks should now be resolved (actually the devel tests now seems fast now)

[adokter]

@bart1 on Linux and Mac, vol2birdR requires certain system requirements to be installed prior to installing the package form R. If we give the simple do you want to install? option I think many people will proceed that without having the system requirements installed. This is why i added the reference to the vol2birdR install instructions in the check_installed message. However the message still tempts people to install without having system dependencies present.

Some options:

1. we could determine what platform people are on, and explicitly instruct them which system dependencies to install first. We need to be able to distinguish between Windows, Mac, Ubuntu and systems supporting yum/rpm, because each has different naming conventions for the same dependencies.
2. Mac/linux OSs are easily checked with .Platform$OS.type=="unix". For those systems we can explicitly warn that system dependencies need to be installed first, without explicitly determining the type of system and referring to the website.
3. for linux and mac remove the option to automatically install altogether, only providing it for windows. We do give the system dependencies worning for linux and mac, like in 2.

Thoughts?

[adokter]

we'll need to warn for required system dependencies for vol2birdR on linux and Mac, and avoid people install without having those present.

[bart1]

Sounds like a valid concern, this is off course an issue for more packages relying on system libraries. The situation is also a bit more complex as more linux precompiled packages come available. I will have to do some research what are common solutions

[peterdesmet]

All this checks and the message to return are best wrapped up in a helper function check_vol2birdR()

[bart1]

I have been thinking a bit more about this. I guess generally system requirements are kind strangely handled in R and it would be hard to completely fix this. There would be two distinct routes of installing that would pose the problem at a different time. Either you would install from source or from a binary (more common now there is the posit repository). For compiling from source you would need to have all -dev libs installed. If you go for binaries only the libraries are required. Normally I would say the correct installation of packages is a concern of the package it self and not package depending on it but given the close link between vol2birdR and bioRad we could see how we can facilitate it (although vol2birdR should of course also work on it self as it is now).

Now for the two installation routes:

• If you install from source you would iteratively encounter the missing dependencies during compilation. I have just tried it with the rocker/tidyverse image. It seems hdf5 and proj missing are caught during configuration but missing gsl and bz2 i encountered only during compilation. It would be good for the configure of vol2bird to at once warn for all missing dependencies (I dont know how difficult that is), now they are encountered one by one.
• If you install from a binary repository (e.g. when vol2birdR is on cran posit might provide this, I'm not sure if they have limits for system dependencies when they do not provide binaries). The installation will proceed without errors. The errors would only occur at run time. I do not have a binary for vol2birdR but for keyring it looks like this:

> rlang::check_installed("keyring")
ℹ The package "keyring" is required.
✖ Would you like to install it?

1: Yes
2: No

Selection: 1
Installing package into ‘/usr/local/lib/R/site-library’
(as ‘lib’ is unspecified)
also installing the dependencies ‘sodium’, ‘filelock’

trying URL 'https://packagemanager.posit.co/cran/__linux__/jammy/latest/src/contrib/sodium_1.2.1.tar.gz'
Content type 'binary/octet-stream' length 500113 bytes (488 KB)
==================================================
downloaded 488 KB

trying URL 'https://packagemanager.posit.co/cran/__linux__/jammy/latest/src/contrib/filelock_1.0.2.tar.gz'
Content type 'binary/octet-stream' length 21806 bytes (21 KB)
==================================================
downloaded 21 KB

trying URL 'https://packagemanager.posit.co/cran/__linux__/jammy/latest/src/contrib/keyring_1.3.1.tar.gz'
Content type 'binary/octet-stream' length 351860 bytes (343 KB)
==================================================
downloaded 343 KB

* installing *binary* package ‘sodium’ ...
* DONE (sodium)
* installing *binary* package ‘filelock’ ...
* DONE (filelock)
* installing *binary* package ‘keyring’ ...
* DONE (keyring)

The downloaded source packages are in
	‘/tmp/RtmpVtYP9k/downloaded_packages’
> require(keyring)
Loading required package: keyring
Error: package or namespace load failed for ‘keyring’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/usr/local/lib/R/site-library/keyring/libs/keyring.so':
  libsecret-1.so.0: cannot open shared object file: No such file or directory

I do not think there is a complete solution. Maybe a message like : For successful installation additionally system dependencies might be required, for details see https://github.com/adokter/vol2birdR#install attached to the reason could work. We could only issue the message on unix like systems.

[bart1]

Now there should be an informative message about system requirements

[bart1]

@adokter does OSX on intel not need the system libraries installed for precompiled binaries? Good to know, that is different from linux

[adokter]

@adokter does OSX on intel not need the system libraries installed for precompiled binaries?

Let me test that before we merge this

[adokter]

@bart1 seems to work without having any system libraries installed, except I do get an annoying warning message with the pre-installed binary (which does not disappear when installing the system libraries), see adokter/vol2birdR#50.

Do you know if CRAN has plans to provide M1 mac and linux binaries? I only see Windows and intel mac binaries offered here

[bart1]

@adokter I'm not sure cran is planning binaries for linux, I don't think so, but I use those at POSIT for fast creation of docker containers. As for OSX I do not really keep up with that.

[bart1]

I also directly removed the remote for vol2birdR