Issue 301: Address coverage gaps in bias.R

[seabbs]

This PR address coverage gaps in bias.R as part of addressing #301.

Unfortunately in doing so it appears to have found a few issues. These are:

• bias_sample(): Could not detect integer forecasts correctly and so always used continuous forecasts
• bias_range() was an effective duplicate of bias_quantile(). It was only tested by checking for equivalence against itself (which of course it passed).
• get_prediction_type() did not fail appropriately when supplied with insufficient data to evaluate the prediction. When this occurred it defaulted a continuous prediction.

I have addressed these issues here but it has required a few internal changes and one potentially breaking change.

See @sbfnk point that the documentation for bias_range() and bias_quantile() is out of sync. I have not addressed this here - see #309.

In the README I see different values for bias despite this being a quantile forecast and so not impacted by the bug identified above. As score_quantile() calls bias_range() this could indicate a few things. A bug in the interaction between bias_range() and bias_quantile(). A bug in bias_quantile() that is just coming to light, that has newly been introduced, or that bias_range() had a bug. Will look at this in more detail. - Fixed by @nikosbosse.

I have fixed linting issues where changing files has caused them to be flagged.

[seabbs]

@nikosbosse Unless I am very confused get_prediction_type() had quite a few bugs in it. Do you agree looking at what I have here so far?

[sbfnk]

@nikosbosse Unless I am very confused get_prediction_type() had quite a few bugs in it. Do you agree looking at what I have here so far?

Not that you've asked me but I agree - worth adding tests for get_prediction_type with data being a matrix?

[seabbs]

if (isFALSE(all.equal(predictions, as.integer(predictions)))) {

Addressed in ae4a857

[seabbs]

To get the vignette work I have had to hack around the point forecasts only case as this depends on bias_range() in a unique way. I have opened an issue for handling point forecasts as it seems like it is currently brittle #311

hack:

    if (is.na(range[1]) && !any(range[-1] == 0)) {
      range[1] <- 0
    }

[codecov]

Codecov Report

Merging #305 (9f3ec52) into main (a493b4c) will increase coverage by 1.09%.
The diff coverage is 91.83%.

❗ Current head 9f3ec52 differs from pull request most recent head 35d7e51. Consider uploading reports for the commit 35d7e51 to get more accurate results

@@            Coverage Diff             @@
##             main     #305      +/-   ##
==========================================
+ Coverage   89.68%   90.78%   +1.09%     
==========================================
  Files          22       22              
  Lines        1377     1378       +1     
==========================================
+ Hits         1235     1251      +16     
+ Misses        142      127      -15     

Files Changed	Coverage Δ
R/input-check-helpers.R	79.76% <84.21%> (+3.13%)	⬆️
R/bias.R	93.75% <91.66%> (+18.40%)	⬆️
R/utils.R	90.76% <94.11%> (+0.93%)	⬆️
R/check_forecasts.R	87.61% <100.00%> (-0.11%)	⬇️
R/summarise_scores.R	82.97% <100.00%> (-0.36%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[seabbs]

I think this should be ready for review now. To get this working with some of the edge cases (point forecasts only for example) I have had to insert some hacks. 😆 turned into a lot of work for a 1% increase in coverage!

This PR has made me think more about how we go about software dev on this package. It might be useful to sit down and spend some time talking through it and potentially draw up some agreed practices + a roadmap for changes we could make to reduce technical debt.

[seabbs]

🙏🏻 📿 🙏🏻

[nikosbosse]

Overall really nice, thank you! Left only a few comments. I made some suggested changes updating the docs (the Latex points Seb mentioned, but haven't made suggested edits for all of them)

[nikosbosse]

It's an important 1% :D Thanks!
And completely agree on the sitting down part and discussing things!