diff --git a/xrspatial/geotiff/__init__.py b/xrspatial/geotiff/__init__.py index f350ddbb..65616028 100644 --- a/xrspatial/geotiff/__init__.py +++ b/xrspatial/geotiff/__init__.py @@ -1360,10 +1360,15 @@ def to_geotiff(data: xr.DataArray | np.ndarray, f"path must be a str or a binary file-like with a write() " f"method, got {type(path).__name__}") + _is_vrt_path = ( + isinstance(path, str) and path.lower().endswith('.vrt')) + # tile_size only applies to tiled output; warn if the caller passed a # non-default size alongside strip mode (it would otherwise be silently - # ignored). - if not tiled and tile_size != 256: + # ignored). The VRT path always tiles, so the warning would be + # misleading there -- the VRT branch below rejects tiled=False up front + # instead. + if not tiled and tile_size != 256 and not _is_vrt_path: warnings.warn( f"tile_size={tile_size} is ignored when tiled=False " "(strip layout). Pass tiled=True to use tile_size, or drop " @@ -1373,7 +1378,18 @@ def to_geotiff(data: xr.DataArray | np.ndarray, # VRT tiled output (string paths only -- VRT writes a real .vrt file # plus per-tile GeoTIFFs to a directory) - if isinstance(path, str) and path.lower().endswith('.vrt'): + if _is_vrt_path: + if not tiled: + raise ValueError( + "tiled=False is not compatible with VRT output. " + "VRT writes a directory of tiled GeoTIFFs; pass " + "tiled=True or omit it.") + # The early ``if tiled: _validate_tile_size_arg(tile_size)`` check + # above already validates tile_size when tiled=True, but call it + # here as well so the VRT path stays self-contained against future + # changes to the early-validation gate (no-op on re-entry; either + # raises or returns). + _validate_tile_size_arg(tile_size) if cog: raise ValueError( "cog=True is not compatible with VRT output. " diff --git a/xrspatial/geotiff/tests/test_to_geotiff_vrt_tiled_validation_1862.py b/xrspatial/geotiff/tests/test_to_geotiff_vrt_tiled_validation_1862.py new file mode 100644 index 00000000..ff160b9a --- /dev/null +++ b/xrspatial/geotiff/tests/test_to_geotiff_vrt_tiled_validation_1862.py @@ -0,0 +1,66 @@ +"""Regression tests for issue #1862. + +``to_geotiff(..., '.vrt', tiled=False, tile_size=0)`` previously warned that +``tile_size`` was ignored, then crashed with ``ZeroDivisionError`` inside +``_write_vrt_tiled`` because the VRT writer always tiles. The ``tiled=False`` +flag was never honored on the VRT path, and ``tile_size`` was only validated +when ``tiled=True``, so an invalid ``tile_size=0`` slipped through. + +``to_geotiff`` now refuses ``tiled=False`` for ``.vrt`` paths up front with a +``ValueError``, and validates ``tile_size`` unconditionally on the VRT +branch so callers get a clear error before the writer divides by it. +""" +from __future__ import annotations + +import os + +import numpy as np +import pytest +import xarray as xr + +from xrspatial.geotiff import to_geotiff + + +def _make_da(shape=(64, 64)): + arr = np.arange(np.prod(shape), dtype=np.float32).reshape(shape) + return xr.DataArray(arr, dims=['y', 'x']) + + +def test_vrt_rejects_tiled_false_1862(tmp_path): + """``tiled=False`` is not a valid request for VRT output.""" + da = _make_da() + out = os.path.join(str(tmp_path), 'vrt_tiled_false_1862.vrt') + with pytest.raises(ValueError, match='tiled=False is not compatible'): + to_geotiff(da, out, tiled=False) + + +def test_vrt_tiled_false_zero_tile_size_raises_value_error_1862(tmp_path): + """``tiled=False`` plus ``tile_size=0`` must raise ``ValueError``, + not the previous ``ZeroDivisionError`` from inside the writer.""" + da = _make_da() + out = os.path.join( + str(tmp_path), 'vrt_tiled_false_zero_1862.vrt') + with pytest.raises(ValueError) as exc: + to_geotiff(da, out, tiled=False, tile_size=0) + # Either the tiled=False guard or the tile_size validator may fire + # first; both produce ValueError, never ZeroDivisionError. + assert not isinstance(exc.value, ZeroDivisionError) + + +def test_vrt_zero_tile_size_default_tiled_raises_value_error_1862(tmp_path): + """With the default ``tiled=True``, ``tile_size=0`` must surface from + the shared ``_validate_tile_size`` check, not a deep ``ZeroDivisionError``. + """ + da = _make_da() + out = os.path.join( + str(tmp_path), 'vrt_default_tiled_zero_1862.vrt') + with pytest.raises(ValueError, match='tile_size'): + to_geotiff(da, out, tile_size=0) + + +def test_vrt_default_args_still_succeeds_1862(tmp_path): + """Sanity: the default-args VRT write path is unaffected by the fix.""" + da = _make_da() + out = os.path.join(str(tmp_path), 'vrt_default_1862.vrt') + to_geotiff(da, out) + assert os.path.exists(out)