Skip to content

Commit caaa83b

Browse files
fix: transform/convert dead ends and JSON round-trip (#537)
Fixes #537. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4d870f0 commit caaa83b

5 files changed

Lines changed: 155 additions & 9 deletions

File tree

src/sktime_mcp/data/adapters/file_adapter.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,11 @@ def load(self) -> pd.DataFrame:
7171
df = self._load_excel(path)
7272
elif file_format == "parquet":
7373
df = self._load_parquet(path)
74+
elif file_format == "json":
75+
df = self._load_json(path)
7476
else:
7577
raise ValueError(
76-
f"Unsupported format: {file_format}. Supported formats: csv, excel, parquet"
78+
f"Unsupported format: {file_format}. Supported formats: csv, excel, parquet, json"
7779
)
7880

7981
# Set time index
@@ -145,6 +147,7 @@ def _detect_format(self, path: Path) -> str:
145147
".xls": "excel",
146148
".parquet": "parquet",
147149
".pq": "parquet",
150+
".json": "json",
148151
}
149152

150153
file_format = format_map.get(suffix)
@@ -217,6 +220,16 @@ def _load_parquet(self, path: Path) -> pd.DataFrame:
217220

218221
return df
219222

223+
def _load_json(self, path: Path) -> pd.DataFrame:
224+
"""Load a JSON file written by save_data (records orient)."""
225+
json_options = self.config.get("json_options", {})
226+
json_options.setdefault("orient", "records")
227+
try:
228+
df = pd.read_json(path, **json_options)
229+
except Exception as e:
230+
raise ValueError(f"Error reading JSON file: {e}") from e
231+
return df
232+
220233
def validate(self, data: pd.DataFrame) -> tuple[bool, dict[str, Any]]:
221234
"""Validate file data using pandas adapter validation."""
222235
from .pandas_adapter import PandasAdapter

src/sktime_mcp/data/adapters/pandas_adapter.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,15 @@ def validate(self, data: pd.DataFrame) -> tuple[bool, dict[str, Any]]:
173173
missing_pct = (missing_counts / len(data) * 100).round(2)
174174
warnings.append(f"Missing values detected: {missing_pct[missing_pct > 0].to_dict()}")
175175

176-
# Check for duplicate indices
176+
# Check for duplicate indices — a warning, not a hard error, so the
177+
# auto-format step (remove_duplicates) can actually run on the handle.
178+
# Rejecting here made that documented remedy unreachable (BUG-19).
177179
if not isinstance(data.index, pd.MultiIndex) and data.index.duplicated().any():
178-
dup_count = data.index.duplicated().sum()
179-
errors.append(f"Duplicate time indices found: {dup_count} duplicates")
180+
dup_count = int(data.index.duplicated().sum())
181+
warnings.append(
182+
f"Duplicate time indices found: {dup_count}. They will be de-duplicated "
183+
"by auto-format (keeping the first of each)."
184+
)
180185

181186
# Check for monotonic index
182187
if not data.index.is_monotonic_increasing:

src/sktime_mcp/tools/save_data.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,18 @@ def save_data_tool(
110110
abs_path.parent.mkdir(parents=True, exist_ok=True)
111111

112112
# Write
113-
writer = getattr(df, _FORMAT_WRITERS[fmt])
114113
if fmt == "json":
115-
writer(str(abs_path), orient="records", date_format="iso", indent=2)
114+
# records orient drops the index; write it as a "time" column so the
115+
# file round-trips through load_data_source(time_column="time").
116+
out_df = df.copy()
117+
out_df.index = out_df.index.astype(str)
118+
out_df = out_df.reset_index(names="time")
119+
out_df.to_json(str(abs_path), orient="records", indent=2)
116120
elif fmt == "parquet":
117-
writer(str(abs_path))
121+
df.to_parquet(str(abs_path))
118122
else:
119123
# CSV — include the index as a time column
120-
writer(str(abs_path))
124+
df.to_csv(str(abs_path))
121125

122126
return {
123127
"success": True,

src/sktime_mcp/tools/transform_data.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,11 @@ def _action_format(
159159
# ---------------------------------------------------------------------------
160160

161161

162+
# Index-less mtypes: converting a handle to these strips the time index, which
163+
# breaks every downstream tool (inspect/split/format) and fabricates a cutoff.
164+
_INDEXLESS_MTYPES = {"np.ndarray", "numpy3D", "numpyflat", "numpy2D"}
165+
166+
162167
def _action_convert(
163168
executor: Any,
164169
data_handle: str,
@@ -170,8 +175,33 @@ def _action_convert(
170175

171176
from sktime.datatypes import convert_to
172177

178+
if to_mtype in _INDEXLESS_MTYPES:
179+
return {
180+
"success": False,
181+
"error": (
182+
f"'{to_mtype}' has no time index; converting a handle to it breaks "
183+
"inspect_data/split_data and other tools. Convert to a pandas mtype "
184+
"(pd.Series, pd.DataFrame) instead."
185+
),
186+
}
187+
173188
original_mtype = type(y).__name__
174-
converted = convert_to(y, to_type=to_mtype)
189+
try:
190+
converted = convert_to(y, to_type=to_mtype)
191+
except (TypeError, ValueError, KeyError) as e:
192+
msg = str(e)
193+
# sktime raises a multi-paragraph mtype-inference dump for a
194+
# scitype-incompatible target (e.g. Series handle -> pd-multiindex).
195+
if "No valid mtype" in msg or "must be of python type" in msg:
196+
return {
197+
"success": False,
198+
"error": (
199+
f"Cannot convert a {original_mtype} (Series-scitype) handle to "
200+
f"'{to_mtype}'. That target expects a different scitype (e.g. Panel). "
201+
"Use a compatible mtype such as pd.Series or pd.DataFrame."
202+
),
203+
}
204+
return {"success": False, "error": f"Conversion to '{to_mtype}' failed: {msg}"}
175205

176206
# Register as new handle
177207
new_handle = f"data_{uuid.uuid4().hex[:8]}"

tests/test_transform_deadends.py

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
"""transform/convert dead ends and JSON round-trip (#537)."""
2+
3+
import contextlib
4+
import os
5+
import tempfile
6+
7+
import pandas as pd
8+
import pytest
9+
10+
from sktime_mcp.runtime.executor import get_executor
11+
from sktime_mcp.tools.save_data import save_data_tool
12+
from sktime_mcp.tools.transform_data import transform_data_tool
13+
14+
15+
@pytest.fixture
16+
def series_handle():
17+
ex = get_executor()
18+
res = ex.load_data_source(
19+
{
20+
"type": "pandas",
21+
"data": {
22+
"date": [f"2024-{m:02d}-01" for m in range(1, 13)],
23+
"value": [float(i) for i in range(12)],
24+
},
25+
"time_column": "date",
26+
"target_column": "value",
27+
}
28+
)
29+
dh = res["data_handle"]
30+
yield ex, dh
31+
for h in list(ex._data_handles):
32+
if h == dh:
33+
ex._data_handles.pop(h, None)
34+
35+
36+
class TestConvertDeadEnds:
37+
def test_ndarray_target_rejected(self, series_handle):
38+
ex, dh = series_handle
39+
res = transform_data_tool(data_handle=dh, action="convert", to_mtype="np.ndarray")
40+
assert not res["success"]
41+
assert "no time index" in res["error"].lower()
42+
43+
def test_series_to_panel_clean_error(self, series_handle):
44+
ex, dh = series_handle
45+
res = transform_data_tool(data_handle=dh, action="convert", to_mtype="pd-multiindex")
46+
assert not res["success"]
47+
# clean domain message, not a multi-paragraph mtype dump
48+
assert "scitype" in res["error"].lower() or "panel" in res["error"].lower()
49+
assert "No valid mtype" not in res["error"]
50+
51+
def test_valid_convert_still_works(self, series_handle):
52+
ex, dh = series_handle
53+
res = transform_data_tool(data_handle=dh, action="convert", to_mtype="pd.DataFrame")
54+
assert res["success"], res
55+
56+
57+
class TestDuplicateTimestamps:
58+
def test_duplicates_load_and_dedup(self):
59+
ex = get_executor()
60+
res = ex.load_data_source(
61+
{
62+
"type": "pandas",
63+
"data": {
64+
"date": ["2024-01-01", "2024-01-01", "2024-02-01", "2024-03-01"],
65+
"value": [1.0, 1.5, 2.0, 3.0],
66+
},
67+
"time_column": "date",
68+
"target_column": "value",
69+
}
70+
)
71+
try:
72+
# previously this hard-failed with "Duplicate time indices found"
73+
assert res["success"], res
74+
# auto-format removed the duplicate
75+
handle = res["data_handle"]
76+
assert not ex._data_handles[handle]["y"].index.duplicated().any()
77+
finally:
78+
ex._data_handles.pop(res.get("data_handle"), None)
79+
80+
81+
class TestJsonRoundTrip:
82+
def test_json_save_then_load(self, series_handle):
83+
ex, dh = series_handle
84+
with tempfile.TemporaryDirectory() as d:
85+
path = os.path.join(d, "data.json")
86+
saved = save_data_tool(dh, path=path, format="json")
87+
assert saved["success"], saved
88+
loaded = ex.load_data_source(
89+
{"type": "file", "path": path, "time_column": "time", "target_column": "value"}
90+
)
91+
try:
92+
assert loaded["success"], loaded
93+
finally:
94+
ex._data_handles.pop(loaded.get("data_handle"), None)

0 commit comments

Comments
 (0)