Skip to content

Commit

Permalink
default tags in BaseForecaster; added some new tags (#1013)
Browse files Browse the repository at this point in the history
Adds some default tag values in `BaseForecaster` - these are automatically inherited via `_all_tags`.

Also adds two new tags that I anticipate we will need:
* `"X-y-must-have-same-index"` - do `X` and `y` need to have the same index?
* `"enforce-index-type"` argument to `check_X`/`check_y` if needed - this has to be a tag now, see the problem in #1005 .

I've also added these new tags to the extension template.

This PR also:
* corrects a minor bug: mis-spelled tags in the extension template
* adds the new tags to the allowed tags in tests
* removes the requirement for tags to be boolean
  • Loading branch information
fkiraly committed Jun 26, 2021
1 parent 324dbea commit b1e7d32
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 10 deletions.
11 changes: 7 additions & 4 deletions extension_templates/forecasting.py
Expand Up @@ -61,12 +61,15 @@ class MyForecaster(BaseForecaster):
"""

# todo: fill out estimator tags here
# delete the tags that you *didn't* change - these defaults are inherited
_tags = {
"fh_in_fit": True, # is the forecasting horizon already required in fit?
"handles_missing_data": False, # can the estimator handle missing data?
"univariate_only": True, # can the estimator deal with multivariate series?
"requires-fh-in-fit": True, # is forecasting horizon already required in fit?
"handles-missing-data": False, # can estimator handle missing data?
"univariate-only": True, # can estimator deal with multivariate series y?
"X-y-must-have-same-index": True, # can estimator handle different X/y index?
"enforce-index-type": None, # index type that needs to be enforced in X/y
}
# in case of inheritance, concrete class should set all tags
# in case of inheritance, concrete class should typically set tags
# alternatively, descendants can set tags in __init__ (avoid this if possible)

# todo: add any hyper-parameters and components to constructor
Expand Down
9 changes: 9 additions & 0 deletions sktime/forecasting/base/_base.py
Expand Up @@ -58,6 +58,15 @@ class BaseForecaster(BaseEstimator):
forecasters.
"""

# default tag values - these typically make the "safest" assumption
_tags = {
"requires-fh-in-fit": True, # is forecasting horizon already required in fit?
"handles-missing-data": False, # can estimator handle missing data?
"univariate-only": True, # can estimator deal with multivariate series y?
"X-y-must-have-same-index": True, # can estimator handle different X/y index?
"enforce-index-type": None, # index type that needs to be enforced in X/y
}

def __init__(self):
self._is_fitted = False

Expand Down
2 changes: 2 additions & 0 deletions sktime/tests/_config.py
Expand Up @@ -287,6 +287,8 @@
"handles-missing-data",
"skip-inverse-transform",
"requires-fh-in-fit",
"X-y-must-have-same-index",
"enforce-index-type",
)

# These methods should not change the state of the estimator, that is, they should
Expand Down
7 changes: 1 addition & 6 deletions sktime/utils/_testing/estimator_checks.py
Expand Up @@ -129,12 +129,7 @@ def check_estimator_tags(Estimator):
assert hasattr(Estimator, "_all_tags")
all_tags = Estimator._all_tags()
assert isinstance(all_tags, dict)
assert all(
[
isinstance(key, str) and isinstance(value, bool)
for key, value in all_tags.items()
]
)
assert all([isinstance(key, str) for key, _ in all_tags.items()])

if hasattr(Estimator, "_tags"):
tags = Estimator._tags
Expand Down

0 comments on commit b1e7d32

Please sign in to comment.