search

hydrosolutions / riversCentralAsia

Package with helper functions to load, manage and analyze hydrometeorological data from Central Asia
GNU General Public License v3.0
1 stars 1 forks source link

Vignettes #109

Closed mengqi-z closed 2 years ago

mengqi-z commented 2 years ago

Hi @mabesa,

This is for JOSS review https://github.com/openjournals/joss-reviews/issues/4805.

I went through the latest files within the vignettes folder, which is not yet successfully deployed and updated on the github documentation page. Anyway, vignettes covers a good amount of package functions. I enjoyed reading through all the examples. However, I suggest that all the data used in the example codes should be provided right before the codes, either directly as part of the R package data, or as downloadable links. Here are the issues I ran into:

  1. In vignettes/01-description_raw_dara.Rmd, section "Climate Projections", the climate nc file provided for the example (https://www.dropbox.com/s/aw3mwh0ysydgu9j/tas_day_GFDL-ESM4_ssp126_r1i1p1f1_gr1.nc?dl=0) only covers data for 1979, where the code example shows 2012. Please either update the example code, or provide the correct file for the example.

  2. In vignettes/03-RSMinerveIO.Rmd, section "Reading and writing parameters", the file Atbaschy_PAR_postCal.txt is not provided in the description. I later find this file in atbashy_glacier_demo_data folder mentioned in 04-glacier-functions.Rmd, but it would be helpful to provide the dataset link every time any data is used in the example. For example, in section "Reading & plotting results" under vignettes/03-RSMinerveIO.Rmd, download link to atbashy_glacier_demo_data should be included.

  3. In vignettes/03-RSMinerveIO.Rmd, section "Writing a check node file", variable Object_IDs is not defined in the example code.

  4. In vignettes/05-snow-calibration.Rmd, section "Extract observed swe", MA_SR_D_v01_N41_0E77_0_agg_16_WY1999_00_MASK.nc and MA_SR_D_v01_N41_0E77_0_agg_16_WY1999_00_SWE_SCA_POST.nc are used in the example but not provided in atbashy_glacier_demo_data/SNOW/.

  5. In vignettes/05-snow-calibration.Rmd, section "Extract observed swe", when running file2write <- file2write |> add_row(cbind(datesChar, data) |> mutate_all(as.character)) I got this error: Error in data.frame(..., check.names = FALSE) : arguments imply differing number of rows: 5101, 40. I didn't get to run through the code section under "Extract observed swe" to produce swel due to reason 4. But I believe I am using swel from SNOW/SWE.RData loaded in the later section. So I am not sure why the length of datesChar and data are not the same. Please check and correct if needed.

Thank you.

mabesa commented 2 years ago

Thank you very much for taking the time to go through the examples in such detail. Your input is very valuable!

The site is not successfully deployed again. I had a curl problem when compiling the site offline but through github actions everything is now working as it should.

(Not mentioned here but in https://github.com/openjournals/joss-reviews/issues/4805): I added a mention about the exactextractr package installation which has to be done manually in some systems. I cannot reproduce the need to manually install the package but I should probably follow up on this.

Issues with using external data for examples in the articles. I cannot include the examples as they are in the package because they are too big. The sources for the example data are not easily found in the example text so I added section headers for the examples.

  1. The linked file you mention shows data from 2012 onwards (the example file in the previous section CHELSA covers data from 1979). I added the sources of the example files in the code chunks to make it a bit clearer how the examples can be reproduced.

  2. I added the link to the file in the text directly above the examples and in the example chunks.

  3. I added reading of the example file that was missing here. The example is now complete.

  4. I see the two files in that folder. I will ask a colleague to check if he sees these files and will continue addressing the comments in a second comment. edit: It took me a while but there were indeed these two files missing. I added them and the examples should now run without problem.