Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions xrspatial/geotiff/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand All @@ -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. "
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Loading