-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check function #154
Add check function #154
Conversation
sorry, mixed this up with a few other tweaks. Relevant file is check_forecasts.R |
Codecov Report
@@ Coverage Diff @@
## master #154 +/- ##
==========================================
+ Coverage 48.44% 54.56% +6.12%
==========================================
Files 18 19 +1
Lines 1348 1468 +120
==========================================
+ Hits 653 801 +148
+ Misses 695 667 -28
Continue to review full report at Codecov.
|
tests still missing at the moment (although examples are run). |
Merge branch 'add-check-function' of https://github.com/epiforecasts/scoringutils into add-check-function # Conflicts: # R/check_forecasts.R # man/check_clean_data.Rd
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some inconsistency in using =
vs <-
so it might be worth running styler.
Co-authored-by: Hugo Gruson <Bisaloo@users.noreply.github.com>
Co-authored-by: Hugo Gruson <Bisaloo@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good Nikos. Mostly just need to resolve @Bisaloo points.
One comment: If the end goal is to have eval_forecasts
be a S3 method itself then at some point the input needs to be assigned a scoringutils class to make that happen. One way you could do this is to assign a class here for each data type as well as your check type and then return the data and checks in a list.
This as a workflow would then be something like this:
data |>
check_forecasts() |>
eval_forecasts()
or
checked_data <- check_forecasts(data)
print(checked_data)
eval_forecasts(checked_data)
Co-authored-by: Hugo Gruson <Bisaloo@users.noreply.github.com>
My idea re S3 classes would be this:
At the moment you can run |
Adds a function to check the input for
eval_forecasts()
.Returns a list with different checks