Functions to export or make internal

[nikosbosse]

Functions we maybe shouldn't export anymore

• as_forecast_generic()
  Argument for making it internal: you only really need the function if you want to create your own forecast class.
• validate_forecast()

Functions we maybe should export

The following is a list of functions that people might appreciate who want to create their own forecast class. They are used by us internally. Not sure how/whether others are supposed to use them.

• new_forecast()
• as_scores()
• apply_metrics()
• ...

Maybe it makes sense to keep them internal for now until we actually see clear need for them?

[seabbs]

you only really need the function if you want to create your own forecast class.

If people want to do this in their own packages they will need this to be exported. I think this could be suited to as_forecast (i.e we enforce all objects to have both a specific and a generic forecast class?) but I don't think we need to do that now.

Anything people need to write their own class we should explore I think. I think we probably should have a toy vignette in the near future demonstrating how to reimplement say point scoring and that will guide what should be exported or not (i.e anything it uses needs to be exported).

get_duplicate_forecasts(

The workflow here could be that users call as_forecast again with a new forecast unit and then pass that to get_duplicate_forecasts?

[nikosbosse]

I'm proposing the following:

For now we make all these functions internal:
-as_forecast_generic()

• new_forecast()
• as_scores()
• apply_metrics()

All these functions are only necessary if you want to build your own new forecast type. Currently that's so convoluted that nobody will do it. In version 2.1 we focus on making that process easier and export those functions.

We could potentially also make

• assert_forecast()
• validate_forecast()
  internal. Those functions are less important now that @Bisaloo implemented automatic warnings when your forecast object becomes invalid.

[seabbs]

I'm not at all clear why we would make functions internal that we know in one minor release time we will need to make external?

[nikosbosse]

More time for thinking vs. committing to maintaining stuff in the future mostly.

[seabbs]

As in we think we might want to make further breaking changes :(

I really hope not. Do you think that is the case for all of these? We do need all of them for a new custom class right so we kind of have to support them (and if eventually remove manage depreciation etc.)

[nikosbosse]

Yeah at some point we probably need to export the functions for creating new forecast classes.

Re validate_forecast() and assert_forecast(): They are kind of duplicate functions. The only difference between the two is that

• assert_forecast() returns NULL invisibly and
• validate_forecast() returns the original object

we're using assert_forecast() to check the forecast conforms with the required format, so you definitely need that function when creating a new forecast type.

validate_forecast() is maybe not necessary at all anymore after @Bisaloo's implementation of [.forecast which revalidates the object whenever you change something. In addition, you can always run as_forecast_<type> again to revalidate an existing forecast object.
To clean up the Namespace, validate_forecast() is a good candidate.

[nikosbosse]

Currently internal functions:

• as_forecast_generic()
• as_scores()
• apply_metrics()

Currently exported functions

• new_forecast()
• validate_forecast()
• assert_forecast()

The updated proposal would be

• to delete validate_forecast()

and to probably I guess have the following functions be exported?

• as_forecast_generic()
• as_scores()
• apply_metrics()
• new_forecast()
• assert_forecast()

It's quite a bit of clutter, but 🤷

EDIT: some thoughts from discussions elsewhere:

• we might think about pre-fixing these functions with something
• some refactoring might make some of these functions obsolete