Issue 272: Add an across argument to summarise_scores()#302
Conversation
Codecov Report
@@ Coverage Diff @@
## main #302 +/- ##
==========================================
+ Coverage 89.61% 89.68% +0.07%
==========================================
Files 22 22
Lines 1367 1377 +10
==========================================
+ Hits 1225 1235 +10
Misses 142 142
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
nikosbosse
left a comment
There was a problem hiding this comment.
Thanks! Looks good to me. Only minor point is that we might want to add a warning if there are column names in across that aren't in the data.
|
I extended the do any of the across variables check to cover are any not in the forecast unit. I also added some verbosity to the documentation. |
|
Nice, looks good! There is just one word or something in the documentation missing, otherwise happy to merge |
|
Reworded. Give it a review and will merge when okay'd |
|
Not sure how strongly you feel about linter complaints :D |
This PR closes #272 by implementing an
acrossargument tosummarise_scores().It also:
summarise_scores()documentation for this new argument.Note that because of the multiple duplicate variables in the example data
acrossisn't actually as useful as you might imagine (i.e. becausetarget_end_dateandhorizondefine the same thing).