Scoringutils 1.0.0 review

[seabbs]

Preface

This is an informal review conducted by a lab member. Following on from #121 the rOpenSci review template has been used.

The template is released under CC-BY-NC-SA and this review is therefore published under the same license.

The review was finished on 2022-03-24 and concerns the version 1.0.0 of scoringutils (commit d30fe10 up to commit f2b01cb)

Package 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

The goal of this package is to provide a tested and reliable collection of metrics that can be used for scoring probabilistic forecasts (forecasts with a full predictive distribution, rather than point forecasts). It has a much stronger focus on convenience than e.g. the scoringRules package, which provides a comprehensive collection of proper scoring rules (also used in scoringutils).

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

These appear to be missing from the readme? I'd give install instructions for both the CRAN version, Universe version, dev version and potentially the old version (at least for now). Added these in #182.

• Vignette(s): demonstrating major functionality that runs successfully locally

Contents of these look good but there were quite monolithic and duplicated content from the README. I have made an optional PR splitting these out (#182).

• Function Documentation: for all exported function

  • Documentation is very impressive and useful. I have reviewed, standardised and attempted to reduce duplication in Documentation and example review #189.
  • Personally, I wouldn't use a custom documentation file for the package root and instead use the one generated by roxygen. This is because it duplicates information (the readme, the vignettes etc) and will require maintained to keep the list of functions up to date.
  • Link to data creation scripts in documentation.  #183
  • Option to round the output of summarise_scores.  #185
  • Documentation of #' @param ... additional parameters passed down to lower-level functions. in score #186
  • permutation_test documentation + testing #188

• Examples: (that run successfully locally) for all exported functions

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

DESCRIPTION is up to date and contribution guidelines are present but not linked from the README. #182 adds this in.

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.

• Test coverage for key metrics is pretty good and unit tests look well implemented. There are a few gaps in coverage which I would like to see closed that also bring down the overall average. These are listed in this issue.

  • Improve testing #196

• Packaging guidelines: The package conforms to the rOpenSci packaging guidelines.

Estimated hours spent reviewing: 24

• 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

This is a robust and well-implemented package with clear, helpful, and detailed documentation. It currently lacks some unit test coverage but what has been implemented has been implemented well.

[seabbs]

@nikosbosse it would be good to chat through the outstanding issues flagged here. In the mean time I will wrap up the functionality and packaging checks once the dev version is merged onto master.

[seabbs]

I've wrapped this up now. It looks great. There are a few issues linked to this that should be fairly easy to deal with before this can be closed.