From 811e64d39dd71d5274d0c45d55cfd86a69f6b1e7 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Thu, 28 May 2026 08:26:41 -0700 Subject: [PATCH 1/2] rasterize: reject partial width/height overrides instead of silent ignore (#2569) When only one of ``width`` / ``height`` was passed alongside ``like=``, the resolver fell through to the ``like`` branch and dropped the explicit dimension without warning. The docstring promised that ``like`` values "can still be overridden explicitly", so this was a silent contract violation. Raise ``ValueError`` naming which dimension was missing. Deriving the missing dimension from aspect/resolution was rejected because the bounds still come from ``like``, so the x and y pixel resolutions would diverge and the output coords would no longer match the template. Also update the ``width``, ``height``, and ``like`` docstring entries to spell out the both-or-neither contract. --- xrspatial/rasterize.py | 28 +++++- .../tests/test_rasterize_partial_dims_2569.py | 92 +++++++++++++++++++ 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 xrspatial/tests/test_rasterize_partial_dims_2569.py diff --git a/xrspatial/rasterize.py b/xrspatial/rasterize.py index 71c2941de..51a48b854 100644 --- a/xrspatial/rasterize.py +++ b/xrspatial/rasterize.py @@ -3002,10 +3002,14 @@ def rasterize( ``(shapely.geometry, numeric_value)`` pair. width : int, optional Number of columns in the output raster. Required unless - ``resolution`` or ``like`` is given. + ``resolution`` or ``like`` is given. Must be passed together + with ``height``: a partial override (only one of the two) is + rejected with ``ValueError`` rather than silently filling the + missing dimension from ``like`` or ``resolution``. height : int, optional Number of rows in the output raster. Required unless - ``resolution`` or ``like`` is given. + ``resolution`` or ``like`` is given. Must be passed together + with ``width`` (see above). bounds : tuple of (xmin, ymin, xmax, ymax), optional Geographic extent of the output raster. Inferred from the geometries (or ``like``) if omitted. @@ -3048,7 +3052,9 @@ def rasterize( A single float uses the same resolution for both axes. like : xr.DataArray, optional Template raster. Width, height, bounds, and dtype are copied - from this array (any can still be overridden explicitly). + from this array. Bounds and dtype can be overridden one at a + time; width and height must be overridden together (passing + only one raises ``ValueError``). Must have uniformly spaced ``x`` and ``y`` dim coords -- the rasterizer only writes to a regular grid, so a non-uniform ``like`` is rejected with ``ValueError`` rather than silently @@ -3205,6 +3211,22 @@ def rasterize( f"Invalid bounds: xmin ({xmin}) must be < xmax ({xmax}) and " f"ymin ({ymin}) must be < ymax ({ymax})") + # Reject partial width/height: passing only one of the two has no + # well-defined meaning here. When ``like`` is given, the bounds also + # come from the template, so deriving the missing dimension from + # aspect ratio would make the x and y pixel resolutions diverge and + # the output coords would no longer match ``like``. When ``like`` is + # not given, there's nothing to derive from at all. Either way, the + # old code silently fell through to the ``resolution`` or ``like`` + # branch and discarded the explicit dimension without warning. + if (width is None) != (height is None): + missing = 'height' if width is not None else 'width' + given = 'width' if width is not None else 'height' + raise ValueError( + f"{given} was provided but {missing} was not. Pass both " + f"width and height, or neither (and use resolution or like " + f"to size the output).") + # Resolve width/height: explicit > resolution > like if width is not None and height is not None: final_width, final_height = int(width), int(height) diff --git a/xrspatial/tests/test_rasterize_partial_dims_2569.py b/xrspatial/tests/test_rasterize_partial_dims_2569.py new file mode 100644 index 000000000..726e00c53 --- /dev/null +++ b/xrspatial/tests/test_rasterize_partial_dims_2569.py @@ -0,0 +1,92 @@ +"""Tests for issue #2569: partial width/height overrides with ``like=`` +must raise rather than be silently ignored. + +The previous behaviour silently fell through to the ``like`` branch when +only one of ``width`` or ``height`` was provided, so +``rasterize(..., like=template, width=W)`` returned the template's +width with no warning. See the docstring change on ``rasterize`` for +the documented contract: width and height are overridden together, or +not at all. +""" + +import numpy as np +import pytest +import xarray as xr +from shapely.geometry import box + +from xrspatial.rasterize import rasterize + + +def _make_like(width=10, height=10): + """2D template DataArray with georeferenced descending-y coords.""" + x = np.linspace(0.5, width - 0.5, width) + y = np.linspace(height - 0.5, 0.5, height) + data = np.zeros((height, width), dtype=np.float64) + return xr.DataArray( + data, dims=['y', 'x'], coords={'y': y, 'x': x}, + ) + + +class TestPartialDimsWithLike: + """``like=`` + exactly one of ``width`` / ``height`` must raise.""" + + def test_like_plus_both_dims_honored(self): + # Sanity: explicit width AND height override the template size. + like = _make_like(width=10, height=10) + result = rasterize( + [(box(2, 2, 8, 8), 1.0)], like=like, width=5, height=4, + fill=0, + ) + assert result.sizes == {'y': 4, 'x': 5} + + def test_like_plus_only_width_raises(self): + like = _make_like() + with pytest.raises(ValueError, match="height was not"): + rasterize( + [(box(2, 2, 8, 8), 1.0)], like=like, width=2, fill=0, + ) + + def test_like_plus_only_height_raises(self): + like = _make_like() + with pytest.raises(ValueError, match="width was not"): + rasterize( + [(box(2, 2, 8, 8), 1.0)], like=like, height=2, fill=0, + ) + + def test_like_plus_neither_uses_like_grid(self): + like = _make_like(width=10, height=10) + result = rasterize( + [(box(2, 2, 8, 8), 1.0)], like=like, fill=0, + ) + assert result.sizes == {'y': 10, 'x': 10} + # Output coords reuse the template's coords bit-exactly. + np.testing.assert_array_equal( + result.coords['x'].values, like.coords['x'].values, + ) + np.testing.assert_array_equal( + result.coords['y'].values, like.coords['y'].values, + ) + + +class TestPartialDimsNoLike: + """Partial width/height without ``like`` must also raise -- the + fall-through used to land on ``resolution`` or the final "must + specify ..." error, both of which dropped the explicit dimension + without a clear message. + """ + + def test_only_width_with_resolution_raises(self): + # Even with resolution providing the size, a half-specified + # width/height is still a contract violation. + with pytest.raises(ValueError, match="height was not"): + rasterize( + [(box(2, 2, 8, 8), 1.0)], + bounds=(0, 0, 10, 10), width=5, resolution=1.0, fill=0, + ) + + def test_only_height_alone_raises(self): + with pytest.raises(ValueError, match="width was not"): + rasterize( + [(box(2, 2, 8, 8), 1.0)], + bounds=(0, 0, 10, 10), height=5, fill=0, + ) From ce0acc1c0de23587231a01f1d5f49b7c1095caee Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Thu, 28 May 2026 08:29:02 -0700 Subject: [PATCH 2/2] Address review nits: relocate partial-dims check, refine error message (#2569) - Move the (width is None) != (height is None) check to right after merge validation, before _extract_grid_from_like and _parse_input. The check has no dependency on geometries or the template, so the earlier placement surfaces the contract violation without paying the bbox-inference cost on the unhappy path. - Reword the error message to "pass both width and height together, or omit both and supply resolution or like" so it no longer implies resolution alone is sufficient (resolution still needs bounds). --- xrspatial/rasterize.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/xrspatial/rasterize.py b/xrspatial/rasterize.py index 51a48b854..9a5080ec4 100644 --- a/xrspatial/rasterize.py +++ b/xrspatial/rasterize.py @@ -3154,6 +3154,23 @@ def rasterize( raise TypeError( f"merge must be a string or callable, got {type(merge).__name__}") + # Reject partial width/height before any geometry or template work. + # Passing only one of the two has no well-defined meaning here. + # When ``like`` is given, the bounds also come from the template, so + # deriving the missing dimension from aspect ratio would make the x + # and y pixel resolutions diverge and the output coords would no + # longer match ``like``. When ``like`` is not given, there's + # nothing to derive from at all. Either way, the old code silently + # fell through to the ``resolution`` or ``like`` branch and + # discarded the explicit dimension without warning. + if (width is None) != (height is None): + missing = 'height' if width is not None else 'width' + given = 'width' if width is not None else 'height' + raise ValueError( + f"{given} was provided but {missing} was not. Pass both " + f"width and height together, or omit both and supply " + f"resolution or like to size the output.") + # Extract defaults from template raster like_width = like_height = like_bounds = like_dtype = None like_x_coord = like_y_coord = None @@ -3211,22 +3228,6 @@ def rasterize( f"Invalid bounds: xmin ({xmin}) must be < xmax ({xmax}) and " f"ymin ({ymin}) must be < ymax ({ymax})") - # Reject partial width/height: passing only one of the two has no - # well-defined meaning here. When ``like`` is given, the bounds also - # come from the template, so deriving the missing dimension from - # aspect ratio would make the x and y pixel resolutions diverge and - # the output coords would no longer match ``like``. When ``like`` is - # not given, there's nothing to derive from at all. Either way, the - # old code silently fell through to the ``resolution`` or ``like`` - # branch and discarded the explicit dimension without warning. - if (width is None) != (height is None): - missing = 'height' if width is not None else 'width' - given = 'width' if width is not None else 'height' - raise ValueError( - f"{given} was provided but {missing} was not. Pass both " - f"width and height, or neither (and use resolution or like " - f"to size the output).") - # Resolve width/height: explicit > resolution > like if width is not None and height is not None: final_width, final_height = int(width), int(height)