Skip to content

Commit

Permalink
Merge pull request #956 from epiforecasts/fix-printing-issue
Browse files Browse the repository at this point in the history
Remove function that resulted in an error when called from within print.data.table()
  • Loading branch information
nikosbosse authored Oct 31, 2024
2 parents bbdc711 + 195d66b commit c7df01c
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 10 deletions.
14 changes: 12 additions & 2 deletions R/class-forecast.R
Original file line number Diff line number Diff line change
Expand Up @@ -272,17 +272,27 @@ is_forecast <- function(x) {
# undesired situation if we set ...length() > 1
# is.data.table: when [.data.table returns an atomic vector, it's clear it
# cannot be a valid forecast object, and it is likely intended by the user
if (data.table::is.data.table(out) && !is_dt_force_print) {

# in addition, we also check for a maximum length. The reason is that
# print.data.table will internally subset the data.table before printing.
# this subsetting triggers the validation, which is not desired in this case.
# this is a hack and ideally, we'd do things differently.
if (nrow(out) > 30 && data.table::is.data.table(out) && !is_dt_force_print) {
# check whether subset object passes validation
validation <- try(
assert_forecast(forecast = out, verbose = FALSE),
silent = TRUE
)
if (inherits(validation, "try-error")) {
cli_warn(
#nolint start: keyword_quote_linter
c(
"!" = "Error in validating forecast object: {validation}"
"!" = "Error in validating forecast object: {validation}.",
"i" = "Note this error is sometimes related to `data.table`s `print`.
Run {.help [{.fun assert_forecast}](scoringutils::assert_forecast)}
to confirm."
)
#nolint end
)
}
}
Expand Down
15 changes: 15 additions & 0 deletions tests/testthat/test-class-forecast-nominal.R
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,18 @@ test_that("get_metrics.forecast_nominal() works as expected", {
is.list(get_metrics(example_nominal))
)
})


# ==============================================================================
# Printing
# ==============================================================================
test_that("Printing works as expected", {
suppressMessages(
expect_message(
expect_message(
capture.output(print(example_nominal)),
"Forecast type: nominal"
),
"Forecast unit:")
)
})
16 changes: 8 additions & 8 deletions tests/testthat/test-class-forecast.R
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,14 @@ test_that("print() works on forecast_* objects", {
test_that("print() throws the expected messages", {
test <- data.table::copy(example_point)
class(test) <- c("point", "forecast", "data.table", "data.frame")
expect_warning(
suppressMessages(
expect_message(
capture.output(print(test)),
"Could not determine forecast type due to error in validation."
)
),
"Error in validating"

# note that since introducing a length maximum for validation to be triggered,
# we don't throw a warning automatically anymore
suppressMessages(
expect_message(
capture.output(print(test)),
"Could not determine forecast type due to error in validation."
)
)

class(test) <- c("forecast_point", "forecast")
Expand Down

0 comments on commit c7df01c

Please sign in to comment.