Distinguish explicit gpu=True vs auto-detected in fallback warning

Two Copilot review notes on PR #1678:

1. The fallback warning at to_geotiff(..., gpu=True) dispatch hard-coded
   "to_geotiff(gpu=True) fell back to CPU" even when GPU was reached
   through the auto-detect branch (gpu is None and the data is CuPy).
   That tells the caller their explicit flag was dropped when they
   never set one, pointing them at the wrong knob. Capture
   auto_detected_gpu = (gpu is None) before resolving use_gpu and pick
   the wording in a single helper so the ImportError and
   RuntimeError branches stay in lockstep.

2. The inline comment on the ImportError branch claimed
   "cupy or nvCOMP not installed" but write_geotiff_gpu only raises
   ImportError for missing cupy. nvCOMP unavailability is handled
   inside _try_nvcomp_from_device_bufs (returns None, triggering CPU
   compression internally), so it never reaches this except block.
   Reworded the comment to match the actual code path.

Tests:
- Updated existing explicit-gpu assertions to pin the new template.
- Strengthened the auto-detect test with full text assertions.
- Added test_auto_detected_gpu_runtime_error_falls_back_with_warning
  to confirm both branches use the same template under auto-detect.
- Added test_explicit_gpu_false_then_true_uses_explicit_template
  to pin that template selection follows gpu is None, not use_gpu
  (so gpu=True on numpy data still gets the explicit wording).

Narrowing logic and strict-mode (XRSPATIAL_GEOTIFF_STRICT) behaviour
are unchanged.

Author: brendancol

xrspatial/geotiff/__init__.py

@@ -103,6 +103,30 @@ def _geotiff_strict_mode() -> bool:
         'XRSPATIAL_GEOTIFF_STRICT', '').lower() in ('1', 'true', 'yes')
 
 
+def _gpu_fallback_warning_message(auto_detected: bool, exc: BaseException) -> str:
+    """Build the ``to_geotiff`` GPU-to-CPU fallback warning text.
+
+    ``to_geotiff`` reaches the GPU writer two ways: an explicit
+    ``gpu=True`` argument, or the auto-detect branch when ``gpu is
+    None`` and the data lives on a CuPy device. The wording differs
+    because blaming the fallback on a flag the caller never set sends
+    them to fix the wrong thing. Both routes share the exception
+    payload format so callers can grep ``type(e).__name__: e`` either
+    way.
+    """
+    suffix = f"({type(exc).__name__}: {exc})."
+    if auto_detected:
+        return (
+            "Data is on the GPU and was routed to the GPU writer, but "
+            "the writer is unavailable; falling back to CPU and copying "
+            "the array to host. " + suffix
+        )
+    return (
+        "to_geotiff(gpu=True) was requested but the GPU writer is "
+        "unavailable; falling back to CPU. " + suffix
+    )
+
+
 def _wkt_to_epsg(wkt_or_proj: str) -> int | None:
     """Try to extract an EPSG code from a WKT or PROJ string.
 
@@ -1132,7 +1156,11 @@ def to_geotiff(data: xr.DataArray | np.ndarray,
                          max_z_error=max_z_error)
         return
 
-    # Auto-detect GPU data and dispatch to write_geotiff_gpu
+    # Auto-detect GPU data and dispatch to write_geotiff_gpu. ``gpu is
+    # None`` is the implicit "use whatever fits the data" path; preserve
+    # that distinction in the fallback warning below so users who never
+    # set ``gpu=True`` are not told their explicit request was dropped.
+    auto_detected_gpu = gpu is None
     use_gpu = gpu if gpu is not None else _is_gpu_data(data)
     if use_gpu and _path_is_file_like:
         # write_geotiff_gpu's nvCOMP path materialises tile parts and then
@@ -1171,15 +1199,19 @@ def to_geotiff(data: xr.DataArray | np.ndarray,
                               streaming_buffer_bytes=streaming_buffer_bytes)
             return
         except ImportError as e:
-            # cupy or nvCOMP not installed. Fall back to the CPU writer
-            # with a typed warning so callers see that gpu=True (or
-            # auto-detected CuPy data) was not honoured. Strict mode
-            # re-raises so CI can fail loudly on missing GPU stacks.
+            # ``write_geotiff_gpu`` raises ImportError when cupy itself
+            # can't be imported. nvCOMP absence doesn't surface here:
+            # ``_try_nvcomp_from_device_bufs`` returns None when the
+            # library can't load, and the writer drops to CPU
+            # compression internally instead of re-raising. Fall back
+            # to the CPU writer with a typed warning so callers see
+            # that gpu=True (or auto-detected CuPy data) didn't go
+            # through. Strict mode re-raises so CI can fail loudly on
+            # missing GPU stacks.
             if _geotiff_strict_mode():
                 raise
             warnings.warn(
-                f"to_geotiff(gpu=True) fell back to CPU "
-                f"({type(e).__name__}: {e}).",
+                _gpu_fallback_warning_message(auto_detected_gpu, e),
                 GeoTIFFFallbackWarning,
                 stacklevel=2,
             )
@@ -1201,8 +1233,7 @@ def to_geotiff(data: xr.DataArray | np.ndarray,
             if _geotiff_strict_mode():
                 raise
             warnings.warn(
-                f"to_geotiff(gpu=True) fell back to CPU "
-                f"({type(e).__name__}: {e}).",
+                _gpu_fallback_warning_message(auto_detected_gpu, e),
                 GeoTIFFFallbackWarning,
                 stacklevel=2,
             )

xrspatial/geotiff/tests/test_to_geotiff_gpu_fallback_1674.py

@@ -122,12 +122,13 @@ def test_import_error_falls_back_with_warning(
         tmp_path, cpu_data, clear_strict_env, monkeypatch):
     """``ImportError`` from the GPU writer triggers a CPU fallback.
 
-    The user asked for ``gpu=True`` on a system without cupy / nvCOMP.
-    A ``GeoTIFFFallbackWarning`` makes the substitution visible.
+    The user asked for ``gpu=True`` on a system without cupy. A
+    ``GeoTIFFFallbackWarning`` makes the substitution visible and the
+    text names the explicit request so users know which knob to tune.
     """
     from xrspatial.geotiff import to_geotiff
 
-    _patch_gpu_writer_to_raise(monkeypatch, ImportError("no nvCOMP"))
+    _patch_gpu_writer_to_raise(monkeypatch, ImportError("no cupy"))
 
     path = tmp_path / "fallback.tif"
     with warnings.catch_warnings(record=True) as records:
@@ -141,8 +142,11 @@ def test_import_error_falls_back_with_warning(
     ]
     assert len(fallback_warnings) == 1
     msg = str(fallback_warnings[0].message)
+    # Explicit gpu=True wording: blame the request, not the data.
+    assert 'to_geotiff(gpu=True)' in msg
+    assert 'Data is on the GPU' not in msg
     assert 'ImportError' in msg
-    assert 'no nvCOMP' in msg
+    assert 'no cupy' in msg
 
 
 def test_import_error_strict_mode_reraises(
@@ -195,7 +199,12 @@ def test_runtime_error_with_gpu_signal_falls_back(
         if issubclass(w.category, GeoTIFFFallbackWarning)
     ]
     assert len(fallback_warnings) == 1
-    assert 'RuntimeError' in str(fallback_warnings[0].message)
+    text = str(fallback_warnings[0].message)
+    assert 'RuntimeError' in text
+    # Explicit gpu=True branch shares the same template as ImportError;
+    # the auto-detected wording must never appear here.
+    assert 'to_geotiff(gpu=True)' in text
+    assert 'Data is on the GPU' not in text
 
 
 def test_runtime_error_with_gpu_signal_strict_reraises(
@@ -215,14 +224,31 @@ def test_runtime_error_with_gpu_signal_strict_reraises(
 # Auto-detected gpu (from CuPy data): same fallback semantics.
 # ---------------------------------------------------------------------------
 
+def _make_synthetic_gpu_data():
+    """Return a numpy-backed DataArray that ``_is_gpu_data`` will be
+    patched to treat as GPU-resident."""
+    arr = np.arange(64, dtype=np.float32).reshape(8, 8)
+    return xr.DataArray(
+        arr,
+        dims=('y', 'x'),
+        coords={
+            'y': np.arange(8, dtype=np.float64),
+            'x': np.arange(8, dtype=np.float64),
+        },
+        attrs={'crs': 4326},
+    )
+
+
 def test_auto_detected_gpu_fallback_warns(
         tmp_path, clear_strict_env, monkeypatch):
     """When ``gpu`` is auto-detected from CuPy-backed data, the same
     fallback rules apply: ``ImportError`` triggers the warning.
 
     Users whose data was CuPy-backed deserve a warning every time the
     GPU writer failed so they know their array was copied to host
-    before the CPU writer wrote it.
+    before the CPU writer wrote it. The warning text must blame the
+    auto-detect path, not an ``gpu=True`` argument the caller never
+    passed.
     """
     from xrspatial.geotiff import to_geotiff
 
@@ -234,16 +260,7 @@ def test_auto_detected_gpu_fallback_warns(
 
     _patch_gpu_writer_to_raise(monkeypatch, ImportError("no cupy"))
 
-    arr = np.arange(64, dtype=np.float32).reshape(8, 8)
-    da = xr.DataArray(
-        arr,
-        dims=('y', 'x'),
-        coords={
-            'y': np.arange(8, dtype=np.float64),
-            'x': np.arange(8, dtype=np.float64),
-        },
-        attrs={'crs': 4326},
-    )
+    da = _make_synthetic_gpu_data()
 
     path = tmp_path / "auto.tif"
     with warnings.catch_warnings(record=True) as records:
@@ -257,3 +274,75 @@ def test_auto_detected_gpu_fallback_warns(
         if issubclass(w.category, GeoTIFFFallbackWarning)
     ]
     assert len(fallback_warnings) == 1
+    text = str(fallback_warnings[0].message)
+    # Auto-detected branch wording: blame the data, not gpu=True.
+    assert 'Data is on the GPU' in text
+    assert 'to_geotiff(gpu=True)' not in text
+    assert 'ImportError' in text
+    assert 'no cupy' in text
+
+
+def test_auto_detected_gpu_runtime_error_falls_back_with_warning(
+        tmp_path, clear_strict_env, monkeypatch):
+    """Same shape for the ``RuntimeError`` branch under auto-detect.
+
+    Both fallback branches (ImportError, RuntimeError-with-GPU-signal)
+    must use the same template so call sites do not diverge over time.
+    """
+    from xrspatial.geotiff import to_geotiff
+    from xrspatial import geotiff as g
+
+    monkeypatch.setattr(g, '_is_gpu_data', lambda data: True, raising=True)
+    _patch_gpu_writer_to_raise(
+        monkeypatch, RuntimeError("CUDA not available"))
+
+    da = _make_synthetic_gpu_data()
+
+    path = tmp_path / "auto_rt.tif"
+    with warnings.catch_warnings(record=True) as records:
+        warnings.simplefilter("always")
+        to_geotiff(da, str(path))
+
+    assert path.exists()
+    fallback_warnings = [
+        w for w in records
+        if issubclass(w.category, GeoTIFFFallbackWarning)
+    ]
+    assert len(fallback_warnings) == 1
+    text = str(fallback_warnings[0].message)
+    assert 'Data is on the GPU' in text
+    assert 'to_geotiff(gpu=True)' not in text
+    assert 'RuntimeError' in text
+    assert 'CUDA not available' in text
+
+
+def test_explicit_gpu_false_then_true_uses_explicit_template(
+        tmp_path, cpu_data, clear_strict_env, monkeypatch):
+    """``gpu=True`` plus non-CuPy data must use the explicit template
+    even when ``_is_gpu_data`` would return False on its own.
+
+    This pins down that the template is selected from ``gpu is None``,
+    not from the resolved ``use_gpu`` value -- so passing ``gpu=True``
+    on numpy data still attributes the fallback to the explicit flag.
+    """
+    from xrspatial.geotiff import to_geotiff
+    from xrspatial import geotiff as g
+
+    # Even if auto-detect would say "not GPU", the explicit request
+    # should drive the wording.
+    monkeypatch.setattr(g, '_is_gpu_data', lambda data: False, raising=True)
+    _patch_gpu_writer_to_raise(monkeypatch, ImportError("no cupy"))
+
+    path = tmp_path / "explicit.tif"
+    with warnings.catch_warnings(record=True) as records:
+        warnings.simplefilter("always")
+        to_geotiff(cpu_data, str(path), gpu=True)
+
+    fallback_warnings = [
+        w for w in records
+        if issubclass(w.category, GeoTIFFFallbackWarning)
+    ]
+    assert len(fallback_warnings) == 1
+    text = str(fallback_warnings[0].message)
+    assert 'to_geotiff(gpu=True)' in text
+    assert 'Data is on the GPU' not in text