Repository navigation
fix: map NaN/Inf to null in sanitize_for_json - #347
SarthakB11 wants to merge 1 commit into
Conversation
Follow-up to sktime#198. The merged sanitize_for_json returns float(obj) for np.floating, which produces NaN/Inf when the input is np.float64('nan') or np.float64('inf'). These have no JSON representation: json.dumps({'x': float(np.float64('nan'))}, allow_nan=False) # ValueError: Out of range float values are not JSON compliant This bites tools that return metrics on edge-case data (e.g. all-NaN columns producing NaN means, or empty CV folds producing inf MASE). Strict JSON consumers (most HTTP clients, MCP transport via JSON-RPC) reject the resulting payload. Map NaN and Inf to None (JSON null) inside the np.floating branch. Strengthen the existing test_nan to assert the actual contract and add test_inf and test_nan_in_dict cases.
|
Bumping this, still mergeable with no conflicts. Small, isolated fix (RFC 8259 compliance for NaN/Inf) with tests. If it helps, I have two other small open PRs against this same repo (#114, #515) that don't overlap with this one code-wise, happy for a batch look if that's easier than three separate passes. |
|
This is the right place for the fix and it still applies cleanly to current main (311 tests pass). One gap: the check only runs in the np.floating branch, but predict returns builtin Python floats (from Series.to_dict()), so the audit scenario (NaiveForecaster with sp=6 on 4 points, predict(horizon=6)) still emits bare NaN on the wire. Please also handle builtin float (math.isfinite in the already-JSON-safe branch, or one shared helper), add a test with float('nan')/float('inf') inside a dict, and ideally switch the json.dumps call in server.py to allow_nan=False as a guard. With that, we merge. |
|
Had a look at this, the check only fires in the With the patch applied I still get |
Follow-up to #198 (merged today).
Bug
The merged
sanitize_for_jsonreturnsfloat(obj)fornp.floating, which producesnan/infwhen the input isnp.float64('nan')ornp.float64('inf'). Strict JSON has no representation for these:Even with default
allow_nan=True, the output{"x": NaN}is invalid JSON per RFC 8259 — strict consumers (most JSON-RPC parsers, including the one MCP uses over stdio) reject it.When this bites
Real cases that produce NaN/Inf in tool responses:
infMASEFix
Map NaN and Inf to
None(JSONnull) inside thenp.floatingbranch. Five-line change.Tests
Strengthens the existing
test_nan(which previously only verifiedjson.dumps(..., default=str)worked — i.e. the escape hatch, not the contract). Addstest_infandtest_nan_in_dictfor realistic usage.