From 3a2fe96a93c037cd4dad852681b6ec422dece4c8 Mon Sep 17 00:00:00 2001
From: Brendan Collins <brendancol@gmail.com>
Date: Fri, 15 May 2026 13:24:11 -0700
Subject: [PATCH] geotiff: reject (y, x, time) 3D writer inputs (#1972)

(time, y, x) was already caught by _validate_3d_writer_dims (issue
#1812), but the symmetric (y, x, time) slipped through the
(y, x, *) band-position fallback and was silently written as a
3-band TIFF. Round-tripping a temporal stack therefore produced a
file that looked like a multiband raster.

Add _TIME_DIM_NAMES in _runtime.py (time / t / date / datetime /
times / dates) and check the trailing dim against it in
_validate_3d_writer_dims. Known temporal names raise with a message
suggesting isel / mean / rename-to-band; the (y, x, *) fallback
for genuinely unknown trailing dim names stays in place so raw
numpy callers building band-last arrays are not bounced.
---
 xrspatial/geotiff/_runtime.py                 |  6 ++
 xrspatial/geotiff/_validation.py              | 26 ++++--
 .../test_temporal_3d_writer_rejection_1972.py | 83 +++++++++++++++++++
 3 files changed, 109 insertions(+), 6 deletions(-)
 create mode 100644 xrspatial/geotiff/tests/test_temporal_3d_writer_rejection_1972.py

diff --git a/xrspatial/geotiff/_runtime.py b/xrspatial/geotiff/_runtime.py
index 402558c8..01a893e7 100644
--- a/xrspatial/geotiff/_runtime.py
+++ b/xrspatial/geotiff/_runtime.py
@@ -62,6 +62,12 @@
 _Y_DIM_NAMES = ('y', 'lat', 'latitude', 'row')
 _X_DIM_NAMES = ('x', 'lon', 'longitude', 'col')
 
+# Temporal dim names. Used by the 3D writer validator (#1972) to refuse
+# ``(y, x, <temporal>)`` inputs that would otherwise be silently treated
+# as multiband rasters. CF / xarray conventions cover ``time`` and ``t``;
+# the rest match common upstream-pipeline aliases.
+_TIME_DIM_NAMES = ('time', 't', 'date', 'datetime', 'times', 'dates')
+
 
 class GeoTIFFFallbackWarning(UserWarning):
     """Warning emitted when a geotiff helper falls back to a slower path.
diff --git a/xrspatial/geotiff/_validation.py b/xrspatial/geotiff/_validation.py
index 790a3ea3..2d9962c9 100644
--- a/xrspatial/geotiff/_validation.py
+++ b/xrspatial/geotiff/_validation.py
@@ -14,7 +14,7 @@
 import numpy as np
 
 from ._coords import _BAND_DIM_NAMES
-from ._runtime import _X_DIM_NAMES, _Y_DIM_NAMES
+from ._runtime import _TIME_DIM_NAMES, _X_DIM_NAMES, _Y_DIM_NAMES
 
 
 def _validate_3d_writer_dims(dims) -> None:
@@ -43,12 +43,26 @@ def _validate_3d_writer_dims(dims) -> None:
                   and d2 in _BAND_DIM_NAMES)
     if band_layout or yxb_layout:
         return
-    # Bare (y, x, *) or (*, y, x) where the third dim is unnamed but
-    # spatial -- the writer's old behaviour treats the non-spatial axis
-    # as bands. Accept that only when the unknown dim is in the band
-    # position (last), which matches how raw numpy callers typically
-    # build a band-last array.
+    # Bare (y, x, *) where the third dim is unnamed but spatial -- the
+    # writer's old behaviour treats the non-spatial axis as bands.
+    # Accept that only when the unknown dim is in the band position
+    # (last), which matches how raw numpy callers typically build a
+    # band-last array. Refuse known *temporal* dim names so a
+    # ``(y, x, time)`` stack is rejected with a clear error instead of
+    # silently being written as a 3-band TIFF (issue #1972). The
+    # mirror case ``(time, y, x)`` was already caught -- this closes
+    # the asymmetry.
     if d0 in _Y_DIM_NAMES and d1 in _X_DIM_NAMES:
+        if d2 in _TIME_DIM_NAMES:
+            raise ValueError(
+                f"3D writer input has temporal trailing dim {d2!r} in dims "
+                f"{dims!r}. The writer would otherwise treat the time axis "
+                f"as bands and silently write a multiband TIFF. Select a "
+                f"single time slice (e.g. ``data.isel({d2}=0)``), reduce "
+                f"with a stat (``data.mean({d2!r})``), or rename to one of "
+                f"{_BAND_DIM_NAMES} if you really intend the temporal "
+                f"axis to round-trip as TIFF bands (issue #1972)."
+            )
         return
     raise ValueError(
         f"3D writer input has ambiguous dims {dims!r}. Expected "
diff --git a/xrspatial/geotiff/tests/test_temporal_3d_writer_rejection_1972.py b/xrspatial/geotiff/tests/test_temporal_3d_writer_rejection_1972.py
new file mode 100644
index 00000000..d8cec888
--- /dev/null
+++ b/xrspatial/geotiff/tests/test_temporal_3d_writer_rejection_1972.py
@@ -0,0 +1,83 @@
+"""Refuse ``(y, x, <time>)`` 3D writer inputs (#1972).
+
+The existing ``_validate_3d_writer_dims`` (issue #1812) rejected the
+``(time, y, x)`` case, but the symmetric ``(y, x, time)`` slipped
+through the ``(y, x, *)`` band-position fallback and was silently
+written as a 3-band TIFF. This locks the temporal-trailing case down
+across the eager, GPU (import-time-check only), and `.vrt` writer
+entry points.
+"""
+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._validation import _validate_3d_writer_dims
+
+
+@pytest.mark.parametrize(
+    "temporal",
+    ['time', 't', 'date', 'datetime', 'times', 'dates'],
+)
+def test_validate_3d_rejects_yx_temporal(temporal):
+    with pytest.raises(ValueError, match="temporal trailing dim"):
+        _validate_3d_writer_dims(('y', 'x', temporal))
+
+
+@pytest.mark.parametrize(
+    "yx",
+    [('y', 'x'), ('lat', 'lon'), ('latitude', 'longitude'), ('row', 'col')],
+)
+def test_validate_3d_rejects_yx_aliases_with_temporal(yx):
+    with pytest.raises(ValueError, match="temporal trailing dim"):
+        _validate_3d_writer_dims((yx[0], yx[1], 'time'))
+
+
+def test_validate_3d_still_accepts_yx_band():
+    _validate_3d_writer_dims(('y', 'x', 'band'))
+    _validate_3d_writer_dims(('band', 'y', 'x'))
+
+
+def test_validate_3d_still_accepts_unknown_trailing_dim():
+    # The (y, x, *) fallback for raw numpy callers stays in place for
+    # genuinely unknown dim names; only known temporal names trip.
+    _validate_3d_writer_dims(('y', 'x', 'channel'))
+    _validate_3d_writer_dims(('y', 'x', 'foo'))
+
+
+def test_validate_3d_still_rejects_time_y_x():
+    # Leading temporal dim was already rejected (asymmetry that #1972
+    # closes); keep the regression test for it.
+    with pytest.raises(ValueError, match="ambiguous dims"):
+        _validate_3d_writer_dims(('time', 'y', 'x'))
+
+
+def test_to_geotiff_rejects_yxtime_stack():
+    da = xr.DataArray(
+        np.zeros((4, 4, 3), dtype=np.float32),
+        coords={'y': np.arange(4.0), 'x': np.arange(4.0),
+                'time': np.arange(3)},
+        dims=('y', 'x', 'time'),
+    )
+    buf = io.BytesIO()
+    with pytest.raises(ValueError, match="temporal trailing dim"):
+        to_geotiff(da, buf)
+
+
+def test_error_message_suggests_isel_and_band_rename():
+    da = xr.DataArray(
+        np.zeros((4, 4, 3), dtype=np.float32),
+        coords={'y': np.arange(4.0), 'x': np.arange(4.0),
+                'time': np.arange(3)},
+        dims=('y', 'x', 'time'),
+    )
+    buf = io.BytesIO()
+    with pytest.raises(ValueError) as excinfo:
+        to_geotiff(da, buf)
+    msg = str(excinfo.value)
+    assert "isel(time=0)" in msg
+    assert "band" in msg.lower()