From 86425e890343b331a30f41fdc8bc92b00b9f3fc1 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Sun, 6 Sep 2026 07:12:31 -0500 Subject: [PATCH] Summarise the values a field actually has summarize_array called plain min/max/mean, which propagate NaN, so one missing value emptied all three statistics. Measured on a temporal mean of a field masked over half its faces: three faces held finite means and the summary reported min, max and mean all NaN. Land masks are ordinary here, so that was most fields. The same payloads were not valid JSON either. json.dumps writes NaN as the bare token NaN, which no parser is required to accept, so a strict client rejected the whole result rather than the one number. Statistics now skip non-finite entries, report None when nothing is finite, and carry n_finite/n_total only when the two differ, so a complete field costs no extra bytes. --- CHANGELOG.md | 13 ++++ src/uxarray_mcp/state.py | 41 ++++++++++++- tests/test_array_summary.py | 117 ++++++++++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 tests/test_array_summary.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c0f88de..9a0e0a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ uses Semantic Versioning for public releases. ## Unreleased ### Fixed +- Array summaries no longer report NaN for every statistic as soon as one + value is missing. `summarize_array` called plain `min`/`max`/`mean`, which + propagate NaN, so a field masked over half its faces returned `min`, `max` + and `mean` all NaN while three faces held finite values — and land masks are + ordinary in this data, so that was most fields. The statistics now skip + non-finite entries, report `None` rather than NaN when nothing is finite, and + add `n_finite`/`n_total` only when the two differ, so a complete field costs + no extra bytes. An infinity counts as missing rather than as an extreme, + since a maximum of `inf` is not a measurement. +- The same payloads were not valid JSON. `json.dumps` writes NaN as the bare + token `NaN`, which no JSON parser is required to accept, so a strict client + rejected the whole result rather than the one number: measured with + `json.loads(..., parse_constant=...)`, which raised on `{"min": NaN}`. - `temporal_mean` and `anomaly` now say how much time they averaged over. Measured on a 6-element, 12-step file: a variable missing at every step returned `outcome: complete`, `status: complete`, no warning codes and a diff --git a/src/uxarray_mcp/state.py b/src/uxarray_mcp/state.py index eb9bd2f..2a8b3b1 100644 --- a/src/uxarray_mcp/state.py +++ b/src/uxarray_mcp/state.py @@ -14,6 +14,7 @@ from pathlib import Path from typing import Any +import numpy as np import xarray as xr _WRITE_LOCK = threading.RLock() @@ -124,15 +125,49 @@ def summarize_grid(grid: Any) -> dict[str, Any]: def summarize_array(data: xr.DataArray) -> dict[str, Any]: - values = data.values + """Shape, dtype and statistics over the values that are actually there. + + The statistics skip non-finite entries, and say so when they had to. The + plain reductions this used to call propagate NaN, so a single missing + value made ``min``, ``max`` and ``mean`` all NaN -- measured on a temporal + mean whose field was masked over half its faces, where three faces held + finite means and the summary reported none of them. Land masks are + ordinary in this data, so that was most fields. + + ``NaN`` was also going out on the wire. ``json.dumps`` writes it as the + bare token ``NaN``, which is not JSON, and a client parsing strictly + rejects the payload rather than the number. Nothing finite reports + ``None`` instead. + + ``n_finite``/``n_total`` appear only when they differ. A count that is + always equal to the size costs payload on every call and tells the caller + nothing they could not read off ``shape``. + """ + values = np.asarray(data.values) summary: dict[str, Any] = { "dims": list(data.dims), "shape": list(data.shape), "dtype": str(data.dtype), "name": str(data.name) if data.name is not None else None, } - if values.size > 0: - with suppress(Exception): + if values.size == 0: + return summary + + with suppress(Exception): + if np.issubdtype(values.dtype, np.inexact): + finite = np.isfinite(values) + n_finite = int(finite.sum()) + if n_finite != values.size: + summary["n_finite"] = n_finite + summary["n_total"] = int(values.size) + usable = values[finite] + summary["min"] = float(usable.min()) if n_finite else None + summary["max"] = float(usable.max()) if n_finite else None + summary["mean"] = float(usable.mean()) if n_finite else None + else: + # Integers and booleans carry no missing value to skip, and + # datetimes raise on float() -- which the suppression handles the + # same way it did before. summary["min"] = float(values.min()) summary["max"] = float(values.max()) summary["mean"] = float(values.mean()) diff --git a/tests/test_array_summary.py b/tests/test_array_summary.py new file mode 100644 index 0000000..dbf9679 --- /dev/null +++ b/tests/test_array_summary.py @@ -0,0 +1,117 @@ +"""Array summaries describe the values that are there, in valid JSON. + +``summarize_array`` used plain ``min``/``max``/``mean``, which propagate NaN, +so one missing value emptied all three statistics. Measured on a temporal mean +of a field masked over half its faces: three faces held finite means and the +summary reported ``min``, ``max`` and ``mean`` all NaN, as ``outcome: +complete``. Land masks are ordinary in this data, so that was most fields. + +``json.dumps`` writes NaN as the bare token ``NaN``, which is not JSON, so the +same payloads were also unparseable by a strict client. +""" + +from __future__ import annotations + +import json + +import numpy as np +import pytest +import xarray as xr + +from uxarray_mcp.state import summarize_array + + +def _array(values, dims=("n_face",), **kwargs) -> xr.DataArray: + return xr.DataArray(np.asarray(values), dims=dims, **kwargs) + + +def _strict_json(payload) -> str: + """Serialise, refusing the non-JSON constants ``json`` emits by default.""" + + def reject(constant: str) -> None: + raise AssertionError(f"payload carries the non-JSON constant {constant}") + + text = json.dumps(payload) + json.loads(text, parse_constant=reject) + return text + + +def test_a_complete_field_summarises_as_before(): + summary = summarize_array(_array([1.0, 2.0, 3.0])) + assert summary["min"] == pytest.approx(1.0) + assert summary["max"] == pytest.approx(3.0) + assert summary["mean"] == pytest.approx(2.0) + assert summary["shape"] == [3] + # Nothing was skipped, so no count is added to every payload in the server. + assert "n_finite" not in summary + assert "n_total" not in summary + + +def test_a_masked_field_reports_the_values_it_has(): + summary = summarize_array(_array([1.0, np.nan, 3.0])) + assert summary["min"] == pytest.approx(1.0) + assert summary["max"] == pytest.approx(3.0) + assert summary["mean"] == pytest.approx(2.0) + assert summary["n_finite"] == 2 + assert summary["n_total"] == 3 + + +def test_an_infinity_counts_as_missing_rather_than_as_an_extreme(): + # A max of inf is not a measurement, and letting it through would make the + # range meaningless wherever a division by zero reached the field. + summary = summarize_array(_array([1.0, np.inf, 3.0])) + assert summary["max"] == pytest.approx(3.0) + assert summary["n_finite"] == 2 + + +def test_a_field_with_nothing_finite_reports_none_not_nan(): + summary = summarize_array(_array([np.nan, np.nan])) + assert summary["min"] is None + assert summary["max"] is None + assert summary["mean"] is None + assert summary["n_finite"] == 0 + assert summary["n_total"] == 2 + + +def test_a_masked_summary_is_valid_json(): + _strict_json(summarize_array(_array([1.0, np.nan]))) + + +def test_an_empty_summary_is_valid_json(): + _strict_json(summarize_array(_array([np.nan]))) + + +def test_an_empty_array_carries_no_statistics(): + summary = summarize_array(_array(np.array([], dtype=float))) + assert "min" not in summary + assert summary["shape"] == [0] + + +def test_an_integer_field_is_summarised_without_a_finite_mask(): + # Integers carry no missing value to skip, so the counts stay off. + summary = summarize_array(_array(np.array([1, 2, 3], dtype="int64"))) + assert summary["min"] == pytest.approx(1.0) + assert summary["mean"] == pytest.approx(2.0) + assert "n_finite" not in summary + + +def test_a_datetime_field_keeps_the_epoch_numbers_it_always_reported(): + # numpy casts datetime64 straight to nanoseconds since the epoch, so these + # statistics were already being returned and are left alone here. Pinning + # them keeps a later decision about what a time summary should say from + # happening by accident inside a NaN fix. + values = np.array(["2000-01-01", "2000-01-02"], dtype="datetime64[ns]") + summary = summarize_array(_array(values)) + assert summary["dtype"].startswith("datetime64") + assert summary["min"] == pytest.approx(9.466848e17) + assert "n_finite" not in summary + + +def test_a_multidimensional_field_counts_every_element(): + summary = summarize_array( + _array([[1.0, np.nan], [3.0, 4.0]], dims=("time", "n_face")) + ) + assert summary["shape"] == [2, 2] + assert summary["n_finite"] == 3 + assert summary["n_total"] == 4 + assert summary["mean"] == pytest.approx(8.0 / 3.0)