Skip to content

Commit 4d809ea

Browse files
fix: reject non-forecasters and non-numeric targets in evaluate (#535)
Validate object_type == forecaster and a numeric Series y before CV, syncing the async path to reject up front. Fixes #535. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent b2a63d5 commit 4d809ea

2 files changed

Lines changed: 154 additions & 0 deletions

File tree

src/sktime_mcp/tools/evaluate.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,75 @@
1414
logger = logging.getLogger(__name__)
1515

1616

17+
def _validate_evaluate_inputs(executor, estimator_handle: str, y: str) -> dict[str, Any] | None:
18+
"""Reject non-forecasters and non-Series targets before any CV work.
19+
20+
Returns an error dict to short-circuit on, or None when inputs are valid.
21+
Cross-validation here uses an ExpandingWindowSplitter and forecasting
22+
metrics, which only make sense for a forecaster on a single Series.
23+
"""
24+
import pandas as pd
25+
26+
try:
27+
instance = executor._handle_manager.get_instance(estimator_handle)
28+
except KeyError:
29+
return {"success": False, "error": executor._handle_manager.describe_missing(estimator_handle)}
30+
31+
get_tag = getattr(instance, "get_class_tag", None)
32+
obj_type = get_tag("object_type", "") if callable(get_tag) else ""
33+
if obj_type != "forecaster":
34+
return {
35+
"success": False,
36+
"error": (
37+
f"evaluate cross-validates forecasters, but this handle is a "
38+
f"{obj_type or 'non-forecaster object'}. Use call_method for "
39+
"non-forecaster estimators."
40+
),
41+
}
42+
43+
# Resolve y and confirm it is a numeric Series (not Panel/Hierarchical, and
44+
# not categorical label data from a classification dataset).
45+
y_res = executor._resolve_source(y)
46+
if not y_res["success"]:
47+
return y_res
48+
_y = y_res["data"]
49+
try:
50+
from sktime.datatypes import check_is_scitype
51+
52+
is_series = check_is_scitype(_y, scitype="Series", return_metadata=[])[0]
53+
except Exception:
54+
is_series = True # let the downstream run surface an unusual case
55+
if not is_series:
56+
return {
57+
"success": False,
58+
"error": (
59+
"evaluate expects a univariate/Series target, but the given y is Panel "
60+
"or Hierarchical scitype. Forecasting CV is not defined for panel data."
61+
),
62+
}
63+
64+
import numpy as np
65+
66+
if isinstance(_y, pd.Series):
67+
numeric = pd.api.types.is_numeric_dtype(_y)
68+
elif isinstance(_y, pd.DataFrame):
69+
numeric = all(pd.api.types.is_numeric_dtype(_y[c]) for c in _y.columns)
70+
elif isinstance(_y, np.ndarray):
71+
numeric = np.issubdtype(_y.dtype, np.number)
72+
else:
73+
numeric = True
74+
if not numeric:
75+
return {
76+
"success": False,
77+
"error": (
78+
f"evaluate needs a numeric forecasting target, but y '{y}' is non-numeric "
79+
"(categorical/label data — this looks like classification data, not a "
80+
"forecasting series)."
81+
),
82+
}
83+
return None
84+
85+
1786
def evaluate_tool(
1887
estimator_handle: str,
1988
y: str,
@@ -42,6 +111,12 @@ def evaluate_tool(
42111

43112
executor = get_executor()
44113

114+
# Scitype validation up front, so async calls reject synchronously instead
115+
# of burning a background job on invalid inputs (#535).
116+
invalid = _validate_evaluate_inputs(executor, estimator_handle, y)
117+
if invalid is not None:
118+
return invalid
119+
45120
if run_async:
46121
job_manager = get_job_manager()
47122
try:

tests/test_evaluate_scitype.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
"""evaluate must reject non-forecasters and non-Series targets up front (#535).
2+
3+
Previously it attempted cross-validation on a transformer/int handle or on
4+
Panel data, wasting time and (before the error_score fix) masking the result.
5+
"""
6+
7+
import contextlib
8+
9+
import pytest
10+
11+
from sktime_mcp.runtime.executor import get_executor
12+
from sktime_mcp.runtime.handles import get_handle_manager
13+
from sktime_mcp.tools.evaluate import evaluate_tool
14+
from sktime_mcp.tools.instantiate import instantiate_tool
15+
16+
17+
def _release(handle):
18+
with contextlib.suppress(KeyError):
19+
get_handle_manager().release_handle(handle)
20+
21+
22+
def test_rejects_transformer():
23+
res = instantiate_tool(spec="Deseasonalizer()")
24+
handle = res["handle"]
25+
try:
26+
out = evaluate_tool(estimator_handle=handle, y="airline", cv_folds=3)
27+
assert not out["success"]
28+
assert "forecaster" in out["error"].lower()
29+
assert "transformer" in out["error"].lower()
30+
finally:
31+
_release(handle)
32+
33+
34+
def test_rejects_non_estimator():
35+
res = instantiate_tool(spec="42")
36+
handle = res["handle"]
37+
try:
38+
out = evaluate_tool(estimator_handle=handle, y="airline", cv_folds=3)
39+
assert not out["success"]
40+
# int handle has no forecaster object_type -> rejected (here or by scitype)
41+
assert "forecaster" in out["error"].lower() or "series" in out["error"].lower()
42+
finally:
43+
_release(handle)
44+
45+
46+
def test_rejects_classification_dataset_target():
47+
"""basic_motions resolves to categorical labels — not a forecasting series."""
48+
res = instantiate_tool(spec="NaiveForecaster()")
49+
handle = res["handle"]
50+
try:
51+
out = evaluate_tool(estimator_handle=handle, y="basic_motions", cv_folds=3)
52+
assert not out["success"]
53+
err = out["error"].lower()
54+
assert "numeric" in err or "panel" in err or "series" in err or "classification" in err
55+
finally:
56+
_release(handle)
57+
58+
59+
def test_forecaster_on_series_still_works():
60+
res = instantiate_tool(spec="NaiveForecaster(sp=12)")
61+
handle = res["handle"]
62+
try:
63+
out = evaluate_tool(estimator_handle=handle, y="airline", cv_folds=3)
64+
assert out["success"], out
65+
assert out["metrics"]
66+
finally:
67+
_release(handle)
68+
69+
70+
def test_async_rejects_synchronously():
71+
"""Invalid inputs must not burn a background job."""
72+
res = instantiate_tool(spec="Deseasonalizer()")
73+
handle = res["handle"]
74+
try:
75+
out = evaluate_tool(estimator_handle=handle, y="airline", cv_folds=3, run_async=True)
76+
assert not out["success"]
77+
assert "job_id" not in out
78+
finally:
79+
_release(handle)

0 commit comments

Comments
 (0)