Initialize review

[maurolepore]

This PR aims to do a first pass through the reviewers's template of rOpenSci. It should be a good guide to bring this package to the standards our peer-reviewers asked for.

Based on https://devguide.ropensci.org/reviewtemplate.html#reviewtemplate

Documentation

The package includes all the following forms of documentation:

• [x] A statement of need clearly stating problems the software is designed to solve and its target audience in README

x README now seems quite bare bones. The paper should have this already. Can you please add it here too?

• [x] Installation instructions: for the development version of package and any non-standard dependencies in README

• [x] Vignette(s) demonstrating major functionality that runs successfully locally

x See ?usethis::use_article()

• [x] Function Documentation: for all exported functions in R help

At first glance, the documentation seems okay.

• [x] Examples for all exported functions in R Help that run successfully locally

x I tried only one example I it failed. Please review all examples. R CMD check should inform what's broken.

library(allodb)

data(scbi_stem1)
est_params(
  genus = scbi_stem1$genus,
  species = scbi_stem1$species,
  coords = c(-78.2, 38.9)
)
#> Error in data.table(weights, equation_id = names(weights)): could not find function "data.table"

^{Created on 2020-12-13 by the reprex package (v0.3.0)}

• [x] Community guidelines including contribution guidelines in the README or CONTRIBUTING, and DESCRIPTION with URL, BugReports and Maintainer (which may be autogenerated via Authors@R).

...

Functionality

• [x] Installation: Installation succeeds as documented.

x I fail to install:

remotes::install_github("forestgeo/allodb")
#> Using github PAT from envvar GITHUB_PAT
#> Downloading GitHub repo forestgeo/allodb@HEAD
#> Error in utils::download.file(url, path, method = method, quiet = quiet,  : 
#>   download from 'https://api.github.com/repos/forestgeo/allodb/tarball/HEAD' failed

• [x] Functionality: Any functional claims of the software been confirmed.

Not reviewed now.

• [ ] Performance: Any performance claims of the software been confirmed.

Not reviewed now.

• [x] 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.

x The tests/ directory seems to have non-standard tests (usually with the testthat package) and the standard test files don't seem to cover all functions. devtools::test() throws:

Maximum number of failures exceeded; quitting early.
You can increase this number by setting `options(testthat.progress.max_fails)` 

...

[maurolepore]

@cpiponiot, here is my first pass through the package. The template I used has a few more details but this seems enought for now.

Please let me know if you need help with any of these issues. A good reference is https://r-pkgs.org/ (or its first edition).

[cpiponiot]

@maurolepore Thanks a lot for the review, I'll work on the package and will come back to you if I have any questions.

[cpiponiot]

Hi @maurolepore! I started adding some test files (for now I've finished the one for the main function of the package, the get_biomass function). Some files in the test/testthat folder were added a long time ago, in particular the test-data, test-master, and test-type_allodb_master that give errors when run. I don't know what they should be testing and if they're still relevant now, but I don't want to remove them either before knowing why they were added in the first place. Did you add them or do you know if I can remove them or change them? Thank you!

[maurolepore]

Happy 2021! Great to know you are moving forward. I suspect you no longer use these tests and you may delete them. You may find your own way to test the code behaves as you expect.

Specifically:

• test-data

These "regression tests" compare specific datasets against a stored reference, and fail when they are different so you explore why. If the differences in all datasets are expected, you can update all the references with the latest state of the datasets: Set update = TRUE in line 7 then re-run the tests. Then you can set again update = FALSE and the tests should pass. If the differences are unexpected, then you will need to explore why.

Here is one example of an dataset that is now different from its old reference:

If you don't use this system to alert you about changes in the data, you may delete this file.

--

• test-master tests a number of properties of the dataset allodb::master(). Are you still using it? If not you can delete this test-file. If yes, the traceback of the failing test suggests two datasets are being joined by the column equation_id but at least one of them lacks such a column.

--

• test-type_allodb_master tests the function type_allodb_master(). Are you still using it? If not you may delete this test-file. If yes, the problem comes from the function master(), and is again about the column equation_id.

Let me know if you need more details.

[cpiponiot]

Thanks @maurolepore ! I've updated test-data and removed the master() and type_allodb_master() functions (with associated test files) that we don't use anymore. Everything seems to be working fine :)