Skip to content

Commit 0a96aa8

Browse files
fix: normalise loaded handles to PeriodIndex so seasonal forecasters work (#531)
load_data_source produced a DatetimeIndex with freq "MS" (MonthBegin) for monthly data. Seasonal forecasters (sp>1) coerce the index to a PeriodIndex internally via index.to_period(freq) and raise "<MonthBegin> is not supported as period frequency" — so fit succeeded and predict failed, with no signal at fit time and no in-toolset workaround. This also broke evaluate and demo-fit + handle-update (NB-19) for the same reason. Normalise a regular DatetimeIndex to PeriodIndex at load, matching sktime's demo datasets (which carry a PeriodIndex and work). to_period() is called with no argument so pandas maps the offset to its period alias (MS -> "M"); passing the offset string back would re-raise the same error. No-op for irregular indexes, already-PeriodIndex, or non-datetime indexes. Applied in format_data_handle (the auto-format-on-load path) and in the auto-format-disabled fallback. Verified: split_data, save_data, plot_series, inspect_data all still work on the period-indexed handles. Fixes #531; unblocks NB-19. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent cbac09f commit 0a96aa8

2 files changed

Lines changed: 142 additions & 0 deletions

File tree

src/sktime_mcp/runtime/executor.py

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,38 @@ def _get_demo_datasets() -> dict:
4949
return _DEMO_DATASETS
5050

5151

52+
def _to_period_index_if_possible(obj: Any) -> Any:
53+
"""Return *obj* with a ``PeriodIndex`` when its index is a regular datetime index.
54+
55+
Seasonal sktime forecasters coerce the series index to a ``PeriodIndex``
56+
internally (``index.to_period(freq)``) and raise on offset frequencies such
57+
as ``MonthBegin`` ("MS"), which is what ``load_data_source`` produces for
58+
monthly data. Demo datasets carry a ``PeriodIndex`` and work, so we
59+
normalise handle-loaded data to match. ``to_period()`` is called with no
60+
argument so pandas maps the offset to its period alias (MS -> "M"); passing
61+
the offset string back in would re-raise the same error.
62+
63+
No-op for non-datetime indexes, ``PeriodIndex`` already, or an index with no
64+
determinable frequency.
65+
"""
66+
if obj is None or not hasattr(obj, "index"):
67+
return obj
68+
idx = obj.index
69+
if isinstance(idx, pd.PeriodIndex) or not isinstance(idx, pd.DatetimeIndex):
70+
return obj
71+
try:
72+
if idx.freq is None:
73+
inferred = pd.infer_freq(idx)
74+
if inferred is None:
75+
return obj
76+
idx = pd.DatetimeIndex(idx, freq=inferred)
77+
converted = obj.copy()
78+
converted.index = idx.to_period()
79+
return converted
80+
except (ValueError, TypeError):
81+
return obj
82+
83+
5284
def _get_index_frequency_metadata(
5385
index: pd.Index,
5486
fallback: str | None = None,
@@ -1055,6 +1087,15 @@ def load_data_source(self, config: dict[str, Any]) -> dict[str, Any]:
10551087
except Exception as e:
10561088
logger.warning(f"Auto-formatting failed: {e}")
10571089
# Continue with unformatted data if formatting fails
1090+
1091+
# Auto-format disabled or failed: still normalise the stored handle to
1092+
# a PeriodIndex where possible so seasonal forecasters work (#531).
1093+
stored = self._data_handles.get(data_handle)
1094+
if stored is not None:
1095+
stored["y"] = _to_period_index_if_possible(stored["y"])
1096+
if stored.get("X") is not None:
1097+
stored["X"] = _to_period_index_if_possible(stored["X"])
1098+
10581099
_final_meta = adapter.get_metadata().copy()
10591100
_final_meta["dtypes"] = {col: str(dtype) for col, dtype in data.dtypes.items()}
10601101
return {
@@ -1302,6 +1343,12 @@ def format_data_handle(
13021343
if X is not None:
13031344
X.index.freq = changes_made["frequency"]
13041345

1346+
# 6. Normalise a regular DatetimeIndex to PeriodIndex so seasonal
1347+
# forecasters can predict on handle-loaded data (#531).
1348+
y = _to_period_index_if_possible(y)
1349+
if X is not None:
1350+
X = _to_period_index_if_possible(X)
1351+
13051352
# Generate new handle
13061353
new_handle = f"data_{uuid.uuid4().hex[:8]}"
13071354

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
"""Handle-loaded monthly data must support seasonal forecasters (#531).
2+
3+
load_data_source produced a DatetimeIndex with freq "MS" (MonthBegin), while
4+
seasonal forecasters coerce to PeriodIndex internally and raise
5+
"<MonthBegin> is not supported as period frequency" at predict time — fit
6+
succeeded, predict failed. Handles are now normalised to PeriodIndex at load,
7+
matching demo datasets.
8+
"""
9+
10+
import pandas as pd
11+
import pytest
12+
13+
from sktime_mcp.runtime.executor import _to_period_index_if_possible, get_executor
14+
from sktime_mcp.tools.fit_predict import fit_tool, predict_tool, update_tool
15+
from sktime_mcp.tools.instantiate import instantiate_tool
16+
17+
18+
def _load_monthly(executor, periods=36):
19+
data = {
20+
"date": [f"20{20 + i // 12}-{i % 12 + 1:02d}-01" for i in range(periods)],
21+
"value": [100.0 + i + 10 * (i % 12) for i in range(periods)],
22+
}
23+
res = executor.load_data_source(
24+
{"type": "pandas", "data": data, "time_column": "date", "target_column": "value"}
25+
)
26+
assert res["success"], res
27+
return res["data_handle"]
28+
29+
30+
class TestHelper:
31+
def test_datetime_ms_becomes_period(self):
32+
idx = pd.date_range("2020-01-01", periods=12, freq="MS")
33+
s = pd.Series(range(12), index=idx)
34+
out = _to_period_index_if_possible(s)
35+
assert isinstance(out.index, pd.PeriodIndex)
36+
assert out.index.freqstr == "M"
37+
38+
def test_freqless_but_regular_is_inferred(self):
39+
idx = pd.DatetimeIndex(pd.date_range("2020-01-01", periods=12, freq="MS").values)
40+
assert idx.freq is None
41+
out = _to_period_index_if_possible(pd.Series(range(12), index=idx))
42+
assert isinstance(out.index, pd.PeriodIndex)
43+
44+
def test_irregular_index_untouched(self):
45+
idx = pd.to_datetime(["2020-01-01", "2020-01-05", "2020-03-02"])
46+
s = pd.Series([1, 2, 3], index=idx)
47+
out = _to_period_index_if_possible(s)
48+
assert isinstance(out.index, pd.DatetimeIndex)
49+
50+
def test_period_index_noop(self):
51+
s = pd.Series(range(6), index=pd.period_range("2020-01", periods=6, freq="M"))
52+
assert _to_period_index_if_possible(s) is not None
53+
assert isinstance(_to_period_index_if_possible(s).index, pd.PeriodIndex)
54+
55+
56+
class TestSeasonalPredictOnHandle:
57+
def test_load_gives_period_index(self):
58+
executor = get_executor()
59+
dh = _load_monthly(executor)
60+
try:
61+
assert isinstance(executor._data_handles[dh]["y"].index, pd.PeriodIndex)
62+
finally:
63+
executor._data_handles.pop(dh, None)
64+
65+
def test_seasonal_fit_then_predict(self):
66+
"""The exact reported repro: fit sp=12 on a handle, then predict."""
67+
executor = get_executor()
68+
dh = _load_monthly(executor)
69+
inst = instantiate_tool(spec="NaiveForecaster(strategy='last', sp=12)")
70+
handle = inst["handle"]
71+
try:
72+
fit_res = fit_tool(estimator_handle=handle, y_handle=dh)
73+
assert fit_res["success"], fit_res
74+
pred = predict_tool(estimator_handle=handle, horizon=6)
75+
assert pred["success"], pred
76+
assert len(pred["predictions"]) == 6
77+
finally:
78+
executor._handle_manager.release_handle(handle)
79+
executor._data_handles.pop(dh, None)
80+
81+
def test_seasonal_evaluate_on_handle(self):
82+
from sktime_mcp.tools.evaluate import evaluate_tool
83+
84+
executor = get_executor()
85+
dh = _load_monthly(executor)
86+
inst = instantiate_tool(spec="NaiveForecaster(sp=12)")
87+
handle = inst["handle"]
88+
try:
89+
res = evaluate_tool(estimator_handle=handle, y=dh, cv_folds=3)
90+
assert res["success"], res
91+
for v in res["metrics"].values():
92+
assert v == v # not NaN
93+
finally:
94+
executor._handle_manager.release_handle(handle)
95+
executor._data_handles.pop(dh, None)

0 commit comments

Comments
 (0)