From 27d9d2ca30f55b29d56beddcce4be02f253e7fc2 Mon Sep 17 00:00:00 2001
From: nikosbosse <nikosbosse@gmail.com>
Date: Thu, 31 Oct 2024 09:47:45 +0100
Subject: [PATCH 1/3] add minimum length requirement to validation by
 subsetting

---
 R/class-forecast.R                           | 12 ++++++++++--
 tests/testthat/test-class-forecast-nominal.R | 10 ++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/R/class-forecast.R b/R/class-forecast.R
index 6992e0a0..a7f00563 100644
--- a/R/class-forecast.R
+++ b/R/class-forecast.R
@@ -272,7 +272,12 @@ 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),
@@ -281,7 +286,10 @@ is_forecast <- function(x) {
     if (inherits(validation, "try-error")) {
       cli_warn(
         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."
         )
       )
     }
diff --git a/tests/testthat/test-class-forecast-nominal.R b/tests/testthat/test-class-forecast-nominal.R
index f6c48fcc..fcc1ecc3 100644
--- a/tests/testthat/test-class-forecast-nominal.R
+++ b/tests/testthat/test-class-forecast-nominal.R
@@ -53,3 +53,13 @@ test_that("get_metrics.forecast_nominal() works as expected", {
     is.list(get_metrics(example_nominal))
   )
 })
+
+
+# ==============================================================================
+# Printing
+# ==============================================================================
+test_that("Printing works as expected", {
+  expect_no_condition(
+    print(example_nominal)
+  )
+})

From b1ee7ba75be39ad12ee9e96b933eb3efc4f4ad40 Mon Sep 17 00:00:00 2001
From: nikosbosse <nikosbosse@gmail.com>
Date: Thu, 31 Oct 2024 10:05:52 +0100
Subject: [PATCH 2/3] update tests

---
 tests/testthat/test-class-forecast-nominal.R |  9 +++++++--
 tests/testthat/test-class-forecast.R         | 16 ++++++++--------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/tests/testthat/test-class-forecast-nominal.R b/tests/testthat/test-class-forecast-nominal.R
index fcc1ecc3..74d5ab52 100644
--- a/tests/testthat/test-class-forecast-nominal.R
+++ b/tests/testthat/test-class-forecast-nominal.R
@@ -59,7 +59,12 @@ test_that("get_metrics.forecast_nominal() works as expected", {
 # Printing
 # ==============================================================================
 test_that("Printing works as expected", {
-  expect_no_condition(
-    print(example_nominal)
+  suppressMessages(
+    expect_message(
+      expect_message(
+        capture.output(print(example_nominal)),
+        "Forecast type: nominal"
+      ),
+      "Forecast unit:")
   )
 })
diff --git a/tests/testthat/test-class-forecast.R b/tests/testthat/test-class-forecast.R
index 74fe1972..67f84967 100644
--- a/tests/testthat/test-class-forecast.R
+++ b/tests/testthat/test-class-forecast.R
@@ -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")

From 195d66b6fc1be246c9e66c7fcdd069362ea453bf Mon Sep 17 00:00:00 2001
From: nikosbosse <nikosbosse@gmail.com>
Date: Thu, 31 Oct 2024 10:19:44 +0100
Subject: [PATCH 3/3] fix linting issue

---
 R/class-forecast.R | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/R/class-forecast.R b/R/class-forecast.R
index a7f00563..a1541f80 100644
--- a/R/class-forecast.R
+++ b/R/class-forecast.R
@@ -285,12 +285,14 @@ is_forecast <- function(x) {
     )
     if (inherits(validation, "try-error")) {
       cli_warn(
+        #nolint start: keyword_quote_linter
         c(
           "!" = "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
       )
     }
   }