geotiff: refuse non-numeric nodata / _FillValue early (#1973)

attrs['_FillValue']='missing' (or attrs['nodata']='missing', or a
non-numeric nodata= kwarg) used to crash deep inside the writer with
``ufunc 'isnan' not supported``. The TypeError did not name the
offending attribute and gave no hint that nodata was the cause.

Add _validate_nodata_arg in _validation.py and call it from
to_geotiff, _write_vrt_tiled, and write_geotiff_gpu so the kwarg
path is checked at the boundary. Tighten _resolve_nodata_attr to
raise ValueError naming the bad attr when attrs['nodata'] or
attrs['_FillValue'] is non-numeric. The rioxarray-style
attrs['nodatavals'] branch keeps its existing skip-on-non-numeric
behaviour (per-band tuples often carry placeholder entries from
arbitrary upstream pipelines, so a single bad entry should not
block writing).

Author: brendancol

xrspatial/geotiff/_attrs.py

@@ -206,6 +206,18 @@ def _resolve_nodata_attr(attrs: dict):
     """
     nodata = attrs.get('nodata')
     if nodata is not None:
+        try:
+            float(nodata)
+        except (TypeError, ValueError) as e:
+            raise ValueError(
+                f"attrs['nodata']={nodata!r} is not numeric "
+                f"({type(nodata).__name__}). The writer needs a numeric "
+                f"sentinel to compare against pixel values; passing a "
+                f"non-numeric value would otherwise crash inside "
+                f"``np.isnan`` with an opaque ufunc error. Drop the "
+                f"attr, replace it with a numeric sentinel, or pass "
+                f"``nodata=`` explicitly (issue #1973)."
+            ) from e
         return nodata
 
     vals = attrs.get('nodatavals')
@@ -229,8 +241,16 @@ def _resolve_nodata_attr(attrs: dict):
     if fill is not None:
         try:
             ffv = float(fill)
-        except (TypeError, ValueError):
-            return fill  # non-numeric -- pass through verbatim
+        except (TypeError, ValueError) as e:
+            raise ValueError(
+                f"attrs['_FillValue']={fill!r} is not numeric "
+                f"({type(fill).__name__}). The writer needs a numeric "
+                f"sentinel to compare against pixel values; passing a "
+                f"non-numeric value would otherwise crash inside "
+                f"``np.isnan`` with an opaque ufunc error. Drop the "
+                f"attr, replace it with a numeric sentinel, or pass "
+                f"``nodata=`` explicitly (issue #1973)."
+            ) from e
         if np.isnan(ffv):
             return None
         return fill

xrspatial/geotiff/_validation.py

@@ -230,3 +230,29 @@ def _validate_predictor_sample_format(predictor, sample_format) -> None:
             f"pair, e.g. `gdal_translate -co PREDICTOR=2` for integers or "
             f"`-co PREDICTOR=1` to drop the predictor."
         )
+
+
+def _validate_nodata_arg(nodata) -> None:
+    """Reject non-numeric ``nodata=`` at the writer entry point (#1973).
+
+    ``None`` (no sentinel) and ``bool`` (kept for backwards compat;
+    rejected at the dtype layer if it does not match) are passed
+    through. Anything else is run through ``float()``: success means
+    the writer's downstream ``np.isnan(nodata)`` and integer-cast
+    paths will not blow up. Failure raises ``ValueError`` with the
+    offending repr, so users see ``nodata='missing'`` flagged at the
+    boundary instead of an opaque ``ufunc 'isnan' not supported``
+    TypeError from inside the writer.
+    """
+    if nodata is None:
+        return
+    try:
+        float(nodata)
+    except (TypeError, ValueError) as e:
+        raise ValueError(
+            f"nodata must be numeric or None, got {nodata!r} "
+            f"(type {type(nodata).__name__}). The writer compares it "
+            f"against pixel values via ``np.isnan`` and casts it to "
+            f"the array dtype; a non-numeric value would otherwise "
+            f"crash inside NumPy with a ufunc TypeError."
+        ) from e

xrspatial/geotiff/_writers/eager.py

@@ -42,6 +42,7 @@
 )
 from .._validation import (
     _validate_3d_writer_dims,
+    _validate_nodata_arg,
     _validate_tile_size_arg,
 )
 from .._writer import write
@@ -267,6 +268,8 @@ def to_geotiff(data: xr.DataArray | np.ndarray,
     if tiled:
         _validate_tile_size_arg(tile_size)
 
+    _validate_nodata_arg(nodata)
+
     # Up-front validation: catch bad compression names before they reach
     # any of the deeper write paths (streaming, GPU, VRT, COG) where the
     # error surfaces from _compression_tag with a less obvious traceback.
@@ -775,6 +778,7 @@ def _write_vrt_tiled(data, vrt_path, *, crs=None, nodata=None,
     This enables streaming dask arrays to disk without materializing the
     full array in RAM.
     """
+    _validate_nodata_arg(nodata)
     # Validate compression_level against codec-specific range
     if compression_level is not None:
         level_range = _LEVEL_RANGES.get(compression.lower())

xrspatial/geotiff/_writers/gpu.py

@@ -27,6 +27,7 @@
 from .._runtime import GeoTIFFFallbackWarning
 from .._validation import (
     _validate_3d_writer_dims,
+    _validate_nodata_arg,
     _validate_tile_size_arg,
 )
 
@@ -259,6 +260,7 @@ def write_geotiff_gpu(data: xr.DataArray | cupy.ndarray | np.ndarray,
     # write_geotiff_gpu is always tiled, so validate tile_size here and
     # keep parity with the public to_geotiff entry point.
     _validate_tile_size_arg(tile_size)
+    _validate_nodata_arg(nodata)
     if max_z_error < 0:
         raise ValueError(
             f"max_z_error must be >= 0, got {max_z_error}")

xrspatial/geotiff/tests/test_nodata_validation_1973.py

@@ -0,0 +1,91 @@
+"""Refuse non-numeric ``nodata=`` / ``attrs['_FillValue']`` (#1973).
+
+The writer compares the resolved nodata against pixel values via
+``np.isnan`` and casts it to the array dtype. A non-numeric value
+used to fall through ``_resolve_nodata_attr`` (returned verbatim) or
+the ``nodata=`` kwarg path and then crash inside NumPy with
+``ufunc 'isnan' not supported``. Both the entry point and the attr
+resolution path now refuse non-numeric values up front with a clear
+error.
+"""
+from __future__ import annotations
+
+import io
+
+import numpy as np
+import pytest
+import xarray as xr
+
+from xrspatial.geotiff import to_geotiff
+from xrspatial.geotiff._attrs import _resolve_nodata_attr
+from xrspatial.geotiff._validation import _validate_nodata_arg
+
+
+def _nan_square():
+    return xr.DataArray(
+        np.full((4, 4), np.nan, dtype=np.float32),
+        coords={'y': np.arange(4.0), 'x': np.arange(4.0)},
+        dims=('y', 'x'),
+    )
+
+
+@pytest.mark.parametrize("bad", ['missing', object(), [1, 2]])
+def test_validate_nodata_arg_rejects_non_numeric(bad):
+    with pytest.raises(ValueError, match="nodata must be numeric"):
+        _validate_nodata_arg(bad)
+
+
+@pytest.mark.parametrize("ok", [None, 0, -9999, 1.5, np.float32(-1), np.int64(0)])
+def test_validate_nodata_arg_accepts_numeric_and_none(ok):
+    _validate_nodata_arg(ok)
+
+
+def test_resolve_nodata_attr_rejects_non_numeric_fillvalue():
+    with pytest.raises(ValueError, match="_FillValue"):
+        _resolve_nodata_attr({'_FillValue': 'missing'})
+
+
+def test_resolve_nodata_attr_rejects_non_numeric_nodata_attr():
+    with pytest.raises(ValueError, match=r"attrs\['nodata'\]"):
+        _resolve_nodata_attr({'nodata': 'missing'})
+
+
+def test_resolve_nodata_attr_skips_non_numeric_in_nodatavals():
+    # nodatavals (rioxarray's per-band tuple) keeps its skip-on-non-numeric
+    # behaviour: those values often come from arbitrary upstream pipelines
+    # and a single bad entry should not block writing.
+    assert _resolve_nodata_attr({'nodatavals': ('NaN ', -9999.0)}) == -9999.0
+
+
+def test_resolve_nodata_attr_still_accepts_numeric_fillvalue():
+    assert _resolve_nodata_attr({'_FillValue': -9999}) == -9999
+
+
+def test_resolve_nodata_attr_returns_none_for_nan_fillvalue():
+    assert _resolve_nodata_attr({'_FillValue': float('nan')}) is None
+
+
+def test_to_geotiff_rejects_non_numeric_nodata_kwarg():
+    buf = io.BytesIO()
+    with pytest.raises(ValueError, match="nodata must be numeric"):
+        to_geotiff(_nan_square(), buf, nodata='missing')
+
+
+def test_to_geotiff_rejects_non_numeric_fillvalue_attr():
+    da = _nan_square()
+    da.attrs['_FillValue'] = 'missing'
+    buf = io.BytesIO()
+    with pytest.raises(ValueError, match="_FillValue"):
+        to_geotiff(da, buf)
+
+
+def test_to_geotiff_vrt_path_rejects_non_numeric_nodata(tmp_path):
+    vrt_path = str(tmp_path / "tmp_1973_vrt.vrt")
+    with pytest.raises(ValueError, match="nodata must be numeric"):
+        to_geotiff(_nan_square(), vrt_path, nodata='missing')
+
+
+def test_to_geotiff_accepts_numeric_nodata_kwarg():
+    buf = io.BytesIO()
+    to_geotiff(_nan_square(), buf, nodata=-9999)
+    assert buf.getbuffer().nbytes > 0