visible data.table return in a few functions

[sbfnk]

Fixes a few (but not all) instances of #129

[codecov]

Codecov Report

Merging #130 (3ccb45c) into master (d09ff23) will increase coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 3ccb45c differs from pull request most recent head 4002705. Consider uploading reports for the commit 4002705 to get more accurate results

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   48.40%   48.43%   +0.03%     
==========================================
  Files          18       18              
  Lines        1345     1346       +1     
==========================================
+ Hits          651      652       +1     
  Misses        694      694              

Impacted Files	Coverage Δ
R/eval_forecasts_binary.R	100.00% <100.00%> (ø)
R/eval_forecasts_continuous_integer.R	69.64% <100.00%> (ø)
R/eval_forecasts_helper.R	100.00% <100.00%> (ø)
R/eval_forecasts_quantile.R	94.05% <100.00%> (ø)
R/utils_data_handling.R	72.64% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d09ff23...4002705. Read the comment docs.

[nikosbosse]

This is something @seabbs and I discussed a year ago. I don't have strong opinions, but I reckon data.table people might. At the moment everything is handled using data.tables and I think for those it is not common practice to call DT[] beforehand.
Admittedly, I also don't really know what the difference is (except for the output to become visible immediately) and why data.tables are implemented the way they are. If we switch this, we should apply the switch to all functions. @Bisaloo do you have thoughts?

[nikosbosse]

Ah ok. Just looked at the issue #129 now and the link you posted (https://cran.r-project.org/web/packages/data.table/vignettes/datatable-faq.html#why-do-i-have-to-type-dt-sometimes-twice-after-using-to-print-the-result-to-console). I wasn't aware that this was a bug in data.table, rather than a feature... Then it makes sense to implement this everywhere. Thanks for pointing this out!

[Bisaloo]

I gave some thought to this and my preferred option would probably be to create a new return_() or myreturn() or scoringutils_return() S3 generic with a specific method for data.table() objects and a default to return() for all other objects.

The main benefit compared to the approach proposed here is that we don't have to check whether the returned object is a data.table object. We use the same function everywhere and the S3 dispatch system will take care of the rest.

[sbfnk]

That's a much more elegant solution - so elegant in fact that I'm surprised that it's not suggested in the data.table FAQ.

[seabbs]

Love it. This would be nice to have in lots of other places so making the solution generic beyond scoringutils would be great.

[nikosbosse]

@Bisaloo this sounds really good. Would you have time to make a PR for this, or shall I give it a try?

[Bisaloo]

I can submit a first draft for this but feel free to edit it as you see fit afterwards.

[nikosbosse]

Merged this PR. I think the solution proposed by @Bisaloo has the problem that a function won't automatically exit if you don't call the standard return() but instead something else...