Fix set forecast unit#437
Conversation
…wasn't working and everything else feels more complicated.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## create-apply-metrics #437 +/- ##
=======================================================
Coverage ? 80.11%
=======================================================
Files ? 20
Lines ? 1705
Branches ? 0
=======================================================
Hits ? 1366
Misses ? 339
Partials ? 0 ☔ View full report in Codecov by Sentry. |
seabbs
left a comment
seabbs
left a comment
There was a problem hiding this comment.
Why isn't this fix targeted at main as it have nothing to do with the current refactor?
Why is there random stuff for run_safely in here?
| missing[[x]] <- x | ||
| } | ||
| } | ||
| missing <- unlist(missing) |
There was a problem hiding this comment.
This appears to be another random change not mentioned in the PR notes?
seabbs
left a comment
seabbs
left a comment
There was a problem hiding this comment.
This is all fine but this should be targetted at main. Are you sure as.data.table doesn't do what you need? https://github.com/epinowcast/epinowcast/blob/d4a65aad2735be7ffa08ca605f979346f62e4212/R/check.R#L165
| #' @keywords internal | ||
| #' @importFrom data.table copy is.data.table as.data.table | ||
| ensure_data.table <- function(data) { | ||
| if (!is.data.table(data)) { |
There was a problem hiding this comment.
linting issues from CI to fix.
There was a problem hiding this comment.
this function reproduces the pattern you have in several places in your code so you should go back and use it if you really need it (vs just using as.data.table which I believe always makes a copy?
There was a problem hiding this comment.
I think I use it everywhere. Reason to use this over as.data.table() is that it preserves other classes. But maybe that's not necessary everywhere.
2 participants
Fixes #427 where
set_forecast_unit()would error if the input is not a data.table.Also the PR
ensure_data.table()that makes sure that the input is a data.tablerun_safely()inmetrics_quantileas that led to an error that I previously didn't catch.