Remove function that resulted in an error when called from within print.data.table()

[nikosbosse]

Description

This PR mostly gives up on #935.

Previously, just calling example_nominal caused an error due to the subsetting that's happening within print.data.table.

This function just removes the function that's responsible for it and adds a test. The issue that manipulating columns of a dt causes random printing still persists, sadly. But at least on error anymore.

Checklist

• [ ] My PR is based on a package issue and I have explicitly linked it.
• [ ] I have included the target issue or issues in the PR title as follows: issue-number: PR title
• [ ] I have tested my changes locally.
• [ ] I have added or updated unit tests where necessary.
• [ ] I have updated the documentation if required.
• [ ] I have built the package locally and run rebuilt docs using roxygen2.
• [ ] My code follows the established coding standards and I have run lintr::lint_package() to check for style issues introduced by my changes.
• [ ] I have added a news item linked to this PR.
• [ ] I have reviewed CI checks for this PR and addressed them as far as I am able.

[sbfnk]

Do we need to make such a drastic change? Can't we allow people to manipulate forecast objects as data.tables but then remove class membership, i.e. replace this warning https://github.com/epiforecasts/scoringutils/blob/bf566e40cb7702341914f35cf872d031c4050ebf/R/class-forecast.R#L341 with

      return(as.data.table(out))

[nikosbosse]

Can't we allow people to manipulate forecast objects as data.tables but then remove class membership

Great idea, will try this!

[nikosbosse]

Can't we allow people to manipulate forecast objects as data.tables but then remove class membership

One downside is that then randomly, at some point, your object will stop being a forecast object and you have no idea when and why that happened...

My impression is that this introduces additional errors (it also causes a whole new set of tests to fail)

[nikosbosse]

Hm. overall I don't have a better idea than removing the subsetting trigger for re-validation. At least assert_forecast() is still exported, so people can still re-validate their forecast objects

[nikosbosse]

@sbfnk let me know if you disagree? Otherwise I'm going to merge this soon-ish

[nikosbosse]

We could do something reaaally hacky and add a filter within [.forecast to only check the content if nrow > 10... or whatever the maximum is that a data.table would print out in any default setting. We'd get weird behaviour, but at least some checking as opposed to none

[seabbs]

Haha I think the print check might on number of rows could be a good partial solution. It's a mega hack as you say obviously.

[nikosbosse]

I implemented a cutoff of 30 now + added a sentence that issues might be due to print and that users should confirm the issue using assert_forecast