Skip to content

Proliferation of forecasting base classes? #510

@fkiraly

Description

@fkiraly

@mloning, @big-o (creditef as contributor in the base classes):

I wanted to work on #464 and looked at the base classes in forecasting, amongst others.
I looked at the base classes and feel there is some unnecessary proliferation of classes going on.

Why: we seem to have two nested template base classes, BaseForecaster and _SktimeForecaster.

The (undocumented) extension pattern seems unnecessarily complicated.
To give one example, implementation of prediction (can you confirm that this is the intended pattern, see new post in #464):

The BaseForecaster expects from extender to inherit and implement predict.
However, if you implement a sktime native forecaster (which most extenders will be concerned with), you have to inherit from _SktimeForecaster, which inherits BaseForecaster and implements predict, but now you have to implement _predict.

I would strongly favour there being only one base class - with all private functions - and a clean docstring that states what to implement.
Only vendor adaptors and similar overrides should have nested template schemes.
Mixins (like for the forecasting horizon) are ok i.m.o.
That way, for example, one wouldn't have to track overrides through two parent classes...

What do you think, @mloning, @big-o? Next major version to refactor this?
Considering use of abc since it's getting a little complex?

Metadata

Metadata

Assignees

No one assigned

    Labels

    API designAPI design & software architecturedocumentationDocumentation & tutorials

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions