chirps: API Client for CHIRPS

[kauedesousa]

Submitting Author: Kauê de Sousa (@kauedesousa)
Repository: https://github.com/agrobioinfoservices/chirps
Version submitted: 0.0.4
Editor: @melvidoni
Reviewer 1: @cvitolo
Reviewer 2: @jzwart
Archive: TBD
Version accepted: TBD

---

• Paste the full DESCRIPTION file inside a code block below:

Package: chirps
Type: Package
Title: API Client for CHIRPS
Version: 0.0.4
Authors@R: c(person("Kaue", "de Sousa", 
        email = "kaue.desousa@inn.no", role = c("aut", "cre"),
        comment = c(ORCID = "0000-0002-7571-7845")), 
        person("Adam H.", "Sparks", role = c("aut"),
        comment = c(ORCID = "0000-0002-0061-8359")),
        person("William", "Ashmall", role = "aut",
        comment = c("API Client implementation")),
        person("Jacob", "van Etten", role = c("ths"),
        comment = c(ORCID = "0000-0001-7554-2558")),
        person("Svein", "Solberg", role = c("ths"),
        comment = c(ORCID = "0000-0002-4491-4483")))
Maintainer: Kaue de Sousa <kaue.desousa@inn.no>
URL: https://agrobioinfoservices.github.io/chirps/
BugReports: https://github.com/agrobioinfoservices/chirps/issues
Description: API Client for the Climate Hazards Group InfraRed Precipitation
  with Station Data 'CHIRPS'. The 'CHIRPS' data is a 35+ year quasi-global
  rainfall data set, which incorporates 0.05 arc-degrees resolution satellite
  imagery, and in-situ station data to create gridded rainfall time series for
  trend analysis and seasonal drought monitoring. For more details on 'CHIRPS'
  data please visit its official home page
  <https://www.chc.ucsb.edu/data/chirps>. 
License: GPL-3
Encoding: UTF-8
LazyData: true
Depends: R (>= 2.10)
Imports:
    crul,
    jsonlite,
    methods,
    sf,
    stats,
    tibble
Suggests:
    knitr,
    rmarkdown,
    testthat (>= 2.1.0)
Language: en-GB
RoxygenNote: 7.0.2
VignetteBuilder: knitr

Scope

• Please indicate which category or categories from our package fit policies this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.):

  • data retrieval
  • data extraction
  • database access
  • data munging
  • data deposition
  • workflow automataion
  • version control
  • citation management and bibliometrics
  • scientific software wrappers
  • database software bindings
  • geospatial data
  • text analysis

• Explain how and why the package falls under these categories (briefly, 1-2 sentences):
  chirps automates downloading and parsing CHIRPS data into a useful format in R using the ClimateSERV API https://climateserv.servirglobal.net/.

• Who is the target audience and what are scientific applications of this package?
  Researchers who use CHIRPS data for modelling or other purpose.

• Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?
  No.

• If you made a pre-submission enquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted.

Technical checks

Confirm each of the following by checking the box. This package:

• does not violate the Terms of Service of any service it interacts with.
• has a CRAN and OSI accepted license.
• contains a README with instructions for installing the development version.
• includes documentation with examples for all functions.
• contains a vignette with examples of its essential functions and uses.
• has a test suite.
• has continuous integration, including reporting of test coverage using services such as Travis CI, Coveralls and/or CodeCov.

Publication options

• Do you intend for this package to go on CRAN?
• Do you wish to automatically submit to the Journal of Open Source Software? If so:

JOSS Options

• The package has an obvious research application according to JOSS's definition.
  • The package contains a paper.md matching JOSS's requirements with a high-level description in the package root or in inst/.
  • The package is deposited in a long-term repository with the DOI:
  • (Do not submit your package separately to JOSS)

• Do you wish to submit an Applications Article about your package to Methods in Ecology and Evolution? If so:

MEE Options

• The package is novel and will be of interest to the broad readership of the journal.
• The manuscript describing the package is no longer than 3000 words.
• You intend to archive the code for the package in a long-term repository which meets the requirements of the journal (see MEE's Policy on Publishing Code)
• (Scope: Do consider MEE's Aims and Scope for your manuscript. We make no guarantee that your manuscript will be within MEE scope.)
• (Although not required, we strongly recommend having a full manuscript prepared when you submit here.)
• (Please do not submit your package separately to Methods in Ecology and Evolution)

Code of conduct

• I agree to abide by rOpenSci's Code of Conduct during the review process and in maintaining my package should it be accepted.

[melvidoni]

Editor checks:

• Fit: The package meets criteria for fit and overlap
• Automated tests: Package has a testing suite and is tested via Travis-CI or another CI service.
• License: The package has a CRAN or OSI accepted license
• Repository: The repository link resolves correctly
• Archive (JOSS only, may be post-review): The repository DOI resolves correctly
• Version (JOSS only, may be post-review): Does the release version given match the GitHub release (v1.0.0)?

---

Editor comments

Thanks for the submission @kauedesousa. This is the result from goodpractice::gp(). Please, correct this and let me know, so I can start searching for reviewers.

── GP chirps ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

It is good practice to

  ✖ avoid long code lines, it is bad for readability. Also, many people prefer editor windows that are about 80 characters wide. Try make your lines
    shorter than 80 characters

    R\GET.R:93:1
    R\internal_functions.R:386:1

  ✖ avoid 1:length(...), 1:nrow(...), 1:ncol(...), 1:NROW(...) and 1:NCOL(...) expressions. They are error prone and result 1:0 if the expression on
    the right hand side is zero. Use seq_len() or seq_along() instead.

    R\GET.R:107:23
    R\get_chirps.R:360:28
    R\get_chirps.R:380:29
    R\get_esi.R:364:28
    R\get_esi.R:391:29
    ... and 4 more lines

  ✖ not import packages as a whole, as this can cause name clashes between the imported packages. Instead, import only the specific functions you
    need.
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── 

---

Reviewers: @cvitolo and @jzwart
Due date: February 10th

[kauedesousa]

Dear @melvidoni thanks for your feedback. I've made the changes and pushed as chirps v0.0.5 AgrDataSci/chirps@95d016f

[melvidoni]

The first reviewer is @cvitolo. I'll fix the review deadline once we have both reviewers.

[melvidoni]

We got a second reviewer: @jzwart
The review deadline is February 10th

[cvitolo]

Hello @kauedesousa!

This is a very useful package, I work in the field of weather forecasting and I greatly appreciate tools that provide an interface to data services. I found your package easy to use and well documented. All your hard work made my review very easy, thanks and great job!

I have some suggestions, which I listed below, I hope you find them useful :)

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

• As the reviewer I confirm that there are no conflicts of interest for me to review this work (If you are unsure whether you are in conflict, please speak to your editor before starting your review).

Documentation

The package includes all the following forms of documentation:

• A statement of need clearly stating problems the software is designed to solve and its target audience in README
• Installation instructions: for the development version of package and any non-standard dependencies in README
• Vignette(s) demonstrating major functionality that runs successfully locally
• Function Documentation: for all exported functions in R help
• Examples for all exported functions in R Help that run successfully locally
• Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

For packages co-submitting to JOSS

• The package has an obvious research application according to JOSS's definition

The package contains a paper.md matching JOSS's requirements with:

• A short summary describing the high-level functionality of the software
• Authors: A list of authors with their affiliations
• A statement of need clearly stating problems the software is designed to solve and its target audience.
• References: with DOIs for all those that have one (e.g. papers, datasets, software).

Functionality

• Installation: Installation succeeds as documented.
• Functionality: Any functional claims of the software been confirmed.
• Performance: Any performance claims of the software been confirmed.
• Automated tests: Unit tests cover essential functions of the package
  and a reasonable range of inputs and conditions. All tests pass on the local machine.
• Packaging guidelines: The package conforms to the rOpenSci packaging guidelines

Final approval (post-review)

• The author has responded to my review and made changes to my satisfaction. I recommend approving this package.

Estimated hours spent reviewing:

• Should the author(s) deem it appropriate, I agree to be acknowledged as a package reviewer ("rev" role) in the package DESCRIPTION file.

---

Review Comments

• README, it seems the code in your README file is only visualised but not executed. In this case, you could keep the README.md and remove README.Rmd (as this is basically redundant).

• Your R folder contains a file called sysdata.rda. Is there a reason to keep these data there?Usually data should be placed under the root folder in a subfolder called data. I would suggest to move sysdata under the chirps/data folder, document the datasets (documentation is currently missing for both tapajos and tapajos_geom) and load the data using data("sysdata") or chirps::tapajos. If you make this change, please change the call to chirps:::tapajo in get_esi() accordingly.

• the man folder contains a figure folder. Is this good practice? I thought the man folder should be used only for documentation. Would it make sense to move the figure folder under inst, for instance?

• tests folder:

  • each of your test files contain a call to library(chirps). This is superflous because you load the package in testthat.R and that should suffice. My suggestion is to load all the packages needed by the tests in testthat.R and remove the commands library(package_name) in the individual test files.
  • When you call library() sometimes you use library(package_name), other times library("package_name"). I would suggest to consistently use the latter.
  • test-get_chirps.R: you only test that you get the right object class but do not test the returned values, is there a reason for that? In my experience, it is very important to test the correctness of the actual data and I would suggest you to develop tests for that.
  • test-get_esi.R: as above, you only test that you get the right object class but do not test the returned values. I would suggest to also test the data values.
  • test-precip_indicesl.R:
    • the file name seems to contain an extra "l", should that not be test-precip_indices.R?
    • you only test the dimensions of the output data frame but not the values themselves. As, above, I would suggest to also test the data values.

• vignettes:

  • The file Overview.Rmd.orig seems redundant and can be removed.
  • It's not necessary to run twice the command precip_indices(dat, timeseries = TRUE, intervals = 15), the second one can be removed.
  • When running the commandget_chirps, I get the following warning message: In st_buffer.sfc(st_geometry(x), dist, nQuadSegs, endCapStyle = endCapStyle,: st_buffer does not correctly buffer longitude/latitude data. Can this warning be eliminated? Maybe adding a note in the documentation?
  • When running the commandget_chirps, I get the following warning message: Warning messages: 1: In st_centroid.sfc(x$geometry) : st_centroid does not give correct centroids for longitude/latitude data. 2: In st_centroid.sfc(x$geometry) : st_centroid does not give correct centroids for longitude/latitude data. Can this warning be eliminated? Maybe adding a note in the documentation?
  • A more in-depth discussion of the functionalities included in the package will make it easier for the reader to understand if the chirps dataset is suitable for a given purpose. I would also mention that requests may take a long time to be executed. Is it feasible to use these functions to download large amount of data (for instance to perform a global scale analysis)? In general, a mention of the limitations of this package would be valuable.

• LICENSE = GPL-3

  • when you use a widely known license you should not need to add a copy of the license to your repo. The files LICENSE and LICENSE.md are redundant and can be removed.
  • When I got my packages reviewed I was made aware that GPL-3 is a strongly protective license and, if you want your package to be used widely (also commercially), MIT or Apache licenses are more suitable. I just wanted to pass on this very valuable suggestion I received.

• inst/paper folder:

  • it seems the code in your paper is only visualised but not executed. In this case, you could keep the paper.md and remove paper.Rmd. Also paper.pdf could be removed.
  • Fig1.svg is redundant (Fig1.png is used for rendering the paper).
  • in the paper, I would move the introduction to the CHIRPS data at the beginning as readers might not be familiar with these data.
  • in the paper you use the command chirps:::tapajos to load data in your sysdata.rda. This is not good practice. The ::: operator should not be used as it exposes non-exported functionalities. If you move sysdata under the chrips/data folder (as suggested above), the dataset can be loaded using data("sysdata") or chirps::tapajos.
  • Towards the end of your paper you state: Overall, these indices proved to be an excellent proxy to evaluate the climate variability using precipitation data [@DeSousa2018], the effects of climate change [@aguilar2005], crop modelling [@Kehel2016] and to define strategies for climate adaptation [@vanEtten2019]. Maybe you could expand a bit, perhaps on the link with crop modelling?

• Lastly, I ran goodpractice::goodpractice() and got 2 messages:

  • write unit tests for all functions, and all package code in general. 34% of code lines are covered by test cases. This differs from what is stated on GitHub (codecv badge = ~73% code coverage). The reason might be due to the fact you skip most of your tests on cran, is this because tests take too long to run? If so, is there a way you could modify the tests so that they take less time?
  • fix this R CMD check WARNING: Missing link or links in documentation object 'precip_indices.Rd': ‘[tidyr]{pivot_wider}’ See section 'Cross-references' in the 'Writing R Extensions' manual. Maybe you could substitute \code{\link[tidyr]{pivot_wider}} with \code{tidyr::pivot_wider()}.

[kauedesousa]

Dear @cvitolo thank you so much for your comments and suggestions. It helped a lot! We have worked in incorporating the comments to chirps v0.0.6 https://github.com/agrobioinfoservices/chirps. Here we have included a point-to-point response to your comments:

general comments

README, it seems the code in your README file is only visualised but not executed. In this case, you could keep the README.md and remove README.Rmd (as this is basically redundant).

The file README.Rmd was removed

Your R folder contains a file called sysdata.rda. Is there a reason to keep these data there? Usually data should be placed under the root folder in a subfolder called data. I would suggest to move sysdata under the chirps/data folder, document the datasets (documentation is currently missing for both tapajos and tapajos_geom) and load the data using data("sysdata") or chirps::tapajos. If you make this change, please change the call to chirps:::tapajo in get_esi() accordingly.

The sf polygon is exported as 'tapajos', the sf POINT object is not necessary and can be generated in the examples with sf. Also, functions dataframe_to_geojson and sf_to_geojson are exported since they had the same issue using ::: in the examples and could be useful for the users

the man folder contains a figure folder. Is this good practice? I thought the man folder should be used only for documentation. Would it make sense to move the figure folder under inst, for instance?

The /man structure is part of using the pre-built vignettes. We not aware of any issues with it. CRAN hasn't baulked at it and Adam (one of the co-authors) just submitted two package updates in the last month that use this method

tests folder:

each of your test files contain a call to library(chirps). This is superflous because you load the package in testthat.R and that should suffice. My suggestion is to load all the packages needed by the tests in testthat.R and remove the commands library(package_name) in the individual test files.

DONE

When you call library() sometimes you use library(package_name), other times library("package_name"). I would suggest to consistently use the latter.

DONE

test-get_chirps.R: you only test that you get the right object class but do not test the returned values, is there a reason for that? In my experience, it is very important to test the correctness of the actual data and I would suggest you to develop tests for that.

We have updated the tests so it checks if the functions return the correct values. For this we downloaded the data from ClimateSERV and compared it with the ones retrieved by get_chirps, get_esi and precip_indices to validate it. The tests still have a skip_on_cran option as a CRAN policy. But we have opened an issue in the package repo and will keep it there until we figure out how to make 'vcr' works with 'chirps' https://github.com/agrobioinfoservices/chirps/issues/7

test-get_esi.R: as above, you only test that you get the right object class but do not test the returned values. I would suggest to also test the data values.

Same as above

test-precip_indicesl.R: the file name seems to contain an extra "l", should that not be test-precip_indices.R? you only test the dimensions of the output data frame but not the values themselves. As, above, I would suggest to also test the data values.

DONE

vignettes:

The file Overview.Rmd.orig seems redundant and can be removed.

We use this file to speed up the vignette creation, here Jeroen Ooms shows how it works https://ropensci.org/technotes/2019/12/08/precompute-vignettes/

It's not necessary to run twice the command precip_indices(dat, timeseries = TRUE, intervals = 15), the second one can be removed.

DONE

When running the command get_chirps, I get the following warning message: In st_buffer.sfc(st_geometry(x), dist, nQuadSegs, endCapStyle = endCapStyle,: st_buffer does not correctly buffer longitude/latitude data. Can this warning be eliminated? Maybe adding a note in the documentation?

These warning messages comes from 'sf', but we don't know if is a good practice to suppress that. We added a note to the documentation

When running the command get_chirps, I get the following warning message: Warning messages: 1: In st_centroid.sfc(x$geometry) : st_centroid does not give correct centroids for longitude/latitude data. 2: In st_centroid.sfc(x$geometry) : st_centroid does not give correct centroids for longitude/latitude data. Can this warning be eliminated? Maybe adding a note in the documentation?

Same as above

more in-depth discussion of the functionalities included in the package will make it easier for the reader to understand if the chirps dataset is suitable for a given purpose. I would also mention that requests may take a long time to be executed. Is it feasible to use these functions to download large amount of data (for instance to perform a global scale analysis)? In general, a mention of the limitations of this package would be valuable.

We added a section for the package limitations. And a better explanation about CHIRPS application into the paper. Also, W. Ashmall says here https://github.com/agrobioinfoservices/chirps/issues/12 that they are planning to upgrade the API service which will make it better to request queries to ClimateSERV.

LICENSE = GPL-3

when you use a widely known license you should not need to add a copy of the license to your repo. The files LICENSE and LICENSE.md are redundant and can be removed.

DONE

When I got my packages reviewed I was made aware that GPL-3 is a strongly protective license and, if you want your package to be used widely (also commercially), MIT or Apache licenses are more suitable. I just wanted to pass on this very valuable suggestion I received.

Thank you, we changed to MIT as suggested

inst/paper folder:

it seems the code in your paper is only visualised but not executed. In this case, you could keep the paper.md and remove paper.Rmd. Also paper.pdf could be removed.

DONE

Fig1.svg is redundant (Fig1.png is used for rendering the paper).

DONE

in the paper, I would move the introduction to the CHIRPS data at the beginning as readers might not be familiar with these data.

DONE

in the paper you use the command chirps:::tapajos to load data in your sysdata.rda. This is not good practice. The ::: operator should not be used as it exposes non-exported functionalities. If you move sysdata under the chrips/data folder (as suggested above), the dataset can be loaded using data("sysdata") or chirps::tapajos.

DONE

Towards the end of your paper you state: Overall, these indices proved to be an excellent proxy to evaluate the climate variability using precipitation data [@DeSousa2018], the effects of climate change [@aguilar2005], crop modelling [@Kehel2016] and to define strategies for climate adaptation [@vanEtten2019]. Maybe you could expand a bit, perhaps on the link with crop modelling?

We updated this section with more examples, and hopefully a better explanation on CHIRPS applications and how chirps can help

goodpractice::goodpractice():

write unit tests for all functions, and all package code in general. 34% of code lines are covered by test cases. This differs from what is stated on GitHub (codecv badge = ~73% code coverage). The reason might be due to the fact you skip most of your tests on cran, is this because tests take too long to run? If so, is there a way you could modify the tests so that they take less time?

Same as above in the tests section

fix this R CMD check WARNING: Missing link or links in documentation object 'precip_indices.Rd': ‘[tidyr]{pivot_wider}’ See section 'Cross-references' in the 'Writing R Extensions' manual. Maybe you could substitute \code{\link[tidyr]{pivot_wider}} with \code{tidyr::pivot_wider()}.

The code was removed from seealso in the documentation.

Again, thank you for your time reviewing this package. We hope you like its new version.

[melvidoni]

Hello @jzwart! Could you submit your review for this package?