search

jackwasey / icd

Fast ICD-10 and ICD-9 comorbidities, decoding and validation in R. NB use main instead of master for default branch.
https://jackwasey.github.io/icd/
GNU General Public License v3.0
240 stars 60 forks source link

dependency on curl executable should be more obvious #193

Open patrickmdnet opened 4 years ago

patrickmdnet commented 4 years ago

I noticed when running icd::download_all_icd_data() on a freshly imaged Windows build (Windows Server 2016 Standard) that I received this error message:

 Error in utils::download.file(url = url, destfile = zipfile, quiet = !.verbose(),  : 
  'curl' call had nonzero exit status

The reason for the error is that there is no "curl.exe" installed and in the PATH of the machine I was using.

Reproduction is easiest by clearing the cache of LIBCIM10MULTI.TXT and running icd:::.dl_icd10fr2019().

I see that icd uses utils::download with method="curl". What would be best is a better error message to the effect that "curl.exe is not present." Also, the documentation should be updated to clarify that curl.exe is a dependency.

I tried using method "auto", but the downloaded zipfile was corrupted somehow. I did not take the time to look into it further. I figure there is a reason you use method="curl".

What are your thoughts?

patrickmdnet commented 4 years ago

Looking at this a little closer, there is a method in to_data_raw.R#L75 called .download that has a bunch of error handling to deal with cross-platform issues with utils::file.download.

The error I saw is triggered by util-file.R#L149 .unzip-single which calls utils::file.download without any of the error handling.

Possible fixes are

  1. change .unzip-single to use the .download method
  2. copy over the necessary error handling from .download into .unzip-single.