Allow users to specify forecast unit variables

[seabbs]

The problem

At the moment the forecast unit is determined automagically within the package and then reported to the user. This works quite well a lot of the time but can lead to some odd edge cases when data changes or when the user is not paying attention. In my own usage I tend to have scoring that initially doesn't work and then have to use check_forecasts() a few times until I get data in the format I want/expect it to be.

Proposed solution

Allow passing forecast_unit explicitly to score() and check_forecasts(). If this is missing default the current behaviour and report the assumed forecast unit when in verbose mode. This would not introduce breaking changes though an ideal solution would add discussion and examples of this usage to the documentation.

Impacts on the rest of the package.

At the moment the automagical determination of forecast unit happens in multiple places. It would make sense to make this happen just once in top-level functions and then require the unit to be passed to lower-level functions (or to be optionally passed as outlined here)

[nikosbosse]

Seems like a good idea. So it could e.g. be passed into ... if the user wants that.
I think it would nevertheless be easiest to remove all columns that are not part of the unit of a single forecast downstream. I remember that in earlier version of scoringutils we had an explicit forecast_unit argument and kept all columns, but I faintly recall that there were a couple of odd edge cases where keeping other columns could have been a problem.

[seabbs]

I think it would nevertheless be easiest to remove all columns that are not part of the unit of a single forecast downstream. I remember that in earlier version of scoringutils we had an explicit forecast_unit argument and kept all column

If we can do this for the user when they specify the forecast unit that might be nice. As I said I have run into this multiple times as a user so can imagine others are also. It very much doesn't reflect the approach most packages take to grouping. I was thinking having it be a named arg vs a ... to make it clear it exists./

[nikosbosse]

No strong preference on it being a named argument or just adding it to the documentation. I remember Hugo feeling strongly about having as few as possible named arguments, but I'm happy to have it be a named argument.

[seabbs]

I think that was in general. I am not sure he is a super big fan of hiding unlimited stuff in ...? @Bisaloo??

[seabbs]

we could also have a helper function to use before score(). Something like define_forecast_unit() that does the dropping for you and is documented?

That would be enough to get me to stop forgetting and having to debug I imagine.

There is the potential that its just me running into this and getting lost. @kathsherratt/@sbfnk have you had a similar issue when using scoringutils?

[seabbs]

This seems related to #294 and its associated PR.

[Bisaloo]

Slightly delayed answer but:

I remember Hugo feeling strongly about having as few as possible named arguments, but I'm happy to have it be a named argument.

I feel strongly about having a few arguments as possible in general, to simplify learning and usage and loosely based on the idea of the "Magical number 7".

But having it explicitly or implicitly as ... leads to the same result so I would prefer having it as an explicit, actual, argument. There could be a case to include it as ... if it's passed to another documented (and ideally exported) function.

[nikosbosse]

#293 addresses this issue by explicitly removing all columns that are not part of the forecast unit. What do you think?

[seabbs]

Looks like a close match to the proposed solution here so makes sense to me.