From 6e2e66bc2b9a33d56bc75d054eb1c84a718d7f80 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Thu, 28 May 2026 08:26:57 -0700 Subject: [PATCH 1/2] rasterize: reject zig-zag duplicate-coord patterns in _check_uniform_axis (#2566) The previous validator compared abs(np.diff(coords)) against abs(expected_step), which accepted patterns like x=[0.5, 1.5, 0.5, 1.5] as "uniform" because every diff magnitude was 1.0. The rasterizer then reused those duplicate coords on the output, so .sel(x=0.5) returned two columns sourced from different burn cells. Switch to comparing signed np.diff against the signed first interval. Ascending and descending axes both still pass (the sign of the first interval carries the direction), but zig-zag / duplicate-coord patterns are rejected with a ValueError naming the offending axis. Adds xrspatial/tests/test_rasterize_signed_step_2566.py covering ascending and descending uniform (pass), zig-zag x and y (rejected), strictly increasing non-uniform (still rejected, the #2168 case), and single-cell axes (short-circuit). --- xrspatial/rasterize.py | 35 +++-- .../tests/test_rasterize_signed_step_2566.py | 140 ++++++++++++++++++ 2 files changed, 162 insertions(+), 13 deletions(-) create mode 100644 xrspatial/tests/test_rasterize_signed_step_2566.py diff --git a/xrspatial/rasterize.py b/xrspatial/rasterize.py index 71c2941de..e94c061a8 100644 --- a/xrspatial/rasterize.py +++ b/xrspatial/rasterize.py @@ -2816,12 +2816,14 @@ def _check_uniform_axis(axis_name, coords, expected_step): than three points cannot be non-uniform in a way this check would catch, so they pass trivially. - The comparison is on ``abs(diff)`` so the validation does not care - whether the axis is ascending or descending -- ascending-y ``like`` - inputs are supported by the orientation flip in ``rasterize``, and - gating on the sign here would block that. ``np.allclose`` is used - (rather than strict equality) because affine-transform-derived - coords drift by a few ulps in practice. + The comparison uses the *signed* ``np.diff`` against the signed + first interval so the validation does not care whether the axis + is ascending or descending -- the sign of the first interval + carries the direction. This also rejects zig-zag / + duplicate-coord patterns like ``[0.5, 1.5, 0.5, 1.5]`` whose + ``abs(diff)`` is uniform but whose signed diffs alternate. + ``np.allclose`` is used (rather than strict equality) because + affine-transform-derived coords drift by a few ulps in practice. """ if coords.size < 3: return @@ -2845,16 +2847,23 @@ def _check_uniform_axis(axis_name, coords, expected_step): "xarray's ``interp`` or ``reindex``) before passing it." ) - diffs = np.abs(np.diff(coords)) - if not np.allclose(diffs, expected_step, rtol=1e-5, atol=1e-8): - max_dev = float(np.max(np.abs(diffs - expected_step))) + # Compare signed diffs, not magnitudes. Comparing only + # ``abs(diff)`` against ``abs(expected_step)`` accepts zig-zag + # patterns like ``[0.5, 1.5, 0.5, 1.5]`` whose magnitudes are + # uniform but whose coords are non-monotonic with duplicate + # values -- which then poisons ``.sel`` and any other coord-aware + # lookup on the output. + signed_step = float(coords[1] - coords[0]) + signed_diffs = np.diff(coords) + if not np.allclose(signed_diffs, signed_step, rtol=1e-5, atol=1e-8): + max_dev = float(np.max(np.abs(signed_diffs - signed_step))) raise ValueError( "'like' DataArray has non-uniform spacing along the " - f"{axis_name!r} axis (expected step {expected_step}, " + f"{axis_name!r} axis (expected step {signed_step}, " f"largest deviation {max_dev}). rasterize() requires a " - "regular grid; resample 'like' to a uniform grid (e.g. " - "with xarray's ``interp`` or ``reindex``) before passing " - "it." + "regular, strictly monotonic grid; resample 'like' to a " + "uniform grid (e.g. with xarray's ``interp`` or " + "``reindex``) before passing it." ) diff --git a/xrspatial/tests/test_rasterize_signed_step_2566.py b/xrspatial/tests/test_rasterize_signed_step_2566.py new file mode 100644 index 000000000..91bb892b0 --- /dev/null +++ b/xrspatial/tests/test_rasterize_signed_step_2566.py @@ -0,0 +1,140 @@ +"""Regression tests for issue #2566. + +``_check_uniform_axis`` in ``xrspatial.rasterize`` previously compared +``abs(np.diff(coords))`` against ``abs(expected_step)``. That accepted +zig-zag / duplicate-coord patterns like ``x = [0.5, 1.5, 0.5, 1.5]`` +as "uniform" because the magnitudes of the diffs were all 1.0. The +rasterizer then reused those duplicate coords on the output, so +``.sel(x=0.5)`` returned two columns whose data came from different +burn cells. + +The fix switches to comparing the *signed* ``np.diff`` against the +signed first interval. Descending axes still pass (a descending y +has a negative signed first interval and all-negative signed diffs), +but zig-zag patterns are rejected with a clear ``ValueError`` naming +the offending axis. + +These tests pin: + +1. Ascending uniform x passes. +2. Descending uniform y passes (the orientation flip in ``rasterize`` + still has to work, so the validator must not gate on sign). +3. Zig-zag duplicate-valued x is rejected with a message naming x. +4. Zig-zag duplicate-valued y is rejected with a message naming y. +5. Strictly increasing but non-uniform spacing is still rejected + (the original #2168 case). +6. Single-cell axes still short-circuit cleanly. +""" + +import numpy as np +import pytest +import xarray as xr +from shapely.geometry import box + +from xrspatial.rasterize import rasterize + + +def _make_like(x, y): + return xr.DataArray( + np.zeros((len(y), len(x)), dtype=np.float64), + dims=('y', 'x'), + coords={'y': np.asarray(y, dtype=np.float64), + 'x': np.asarray(x, dtype=np.float64)}, + ) + + +class TestSignedStepValidation_2566: + """``_check_uniform_axis`` must require signed diffs to match.""" + + def test_ascending_uniform_passes(self): + # Both axes ascending and uniform -- the basic happy path. + x_2566 = np.linspace(0.5, 9.5, 10) + y_2566 = np.linspace(0.5, 9.5, 10) + like_2566 = _make_like(x_2566, y_2566) + # Should not raise. + result = rasterize( + [(box(2, 2, 8, 8), 1.0)], + like=like_2566, fill=0, + ) + assert result.shape == (10, 10) + + def test_descending_uniform_y_passes(self): + # Descending y is the standard image-orientation case. The + # signed first interval is negative; all signed diffs are + # also negative and equal, so the check should accept it. + x_2566 = np.linspace(0.5, 9.5, 10) + y_2566 = np.linspace(9.5, 0.5, 10) + like_2566 = _make_like(x_2566, y_2566) + result = rasterize( + [(box(2, 2, 8, 8), 1.0)], + like=like_2566, fill=0, + ) + assert result.shape == (10, 10) + assert np.any(result.values == 1.0) + + def test_zigzag_x_rejected(self): + # ``x = [0.5, 1.5, 0.5, 1.5]`` -- abs(diff) == [1, 1, 1] but + # signs alternate. Previously accepted; now rejected. + x_2566 = np.array([0.5, 1.5, 0.5, 1.5]) + y_2566 = np.linspace(3.5, 0.5, 4) + like_2566 = _make_like(x_2566, y_2566) + with pytest.raises(ValueError) as excinfo: + rasterize( + [(box(0, 0, 2, 4), 1.0)], + like=like_2566, fill=0, + ) + msg = str(excinfo.value) + assert "'x'" in msg + assert "non-uniform" in msg.lower() + + def test_zigzag_y_rejected(self): + # Same pattern on y. + x_2566 = np.linspace(0.5, 3.5, 4) + y_2566 = np.array([0.5, 1.5, 0.5, 1.5]) + like_2566 = _make_like(x_2566, y_2566) + with pytest.raises(ValueError) as excinfo: + rasterize( + [(box(0, 0, 4, 2), 1.0)], + like=like_2566, fill=0, + ) + msg = str(excinfo.value) + assert "'y'" in msg + assert "non-uniform" in msg.lower() + + def test_strictly_increasing_non_uniform_still_rejected(self): + # The original #2168 case: monotonic but irregular spacing. + # The signed-diff fix must not regress this. + x_2566 = np.array([0.0, 1.0, 2.5, 3.5]) + y_2566 = np.linspace(3.5, 0.5, 4) + like_2566 = _make_like(x_2566, y_2566) + with pytest.raises(ValueError, match=r"'x'"): + rasterize( + [(box(0, 0, 3.5, 4), 1.0)], + like=like_2566, fill=0, + ) + + def test_single_cell_axis_passes(self): + # size < 3 short-circuits; a 2-row template along y should + # still validate without raising. Pair with a uniform x. + x_2566 = np.linspace(0.5, 9.5, 10) + y_2566 = np.array([1.5, 0.5]) + like_2566 = _make_like(x_2566, y_2566) + # Should not raise. + rasterize( + [(box(0, 0, 10, 2), 1.0)], + like=like_2566, fill=0, + ) + + def test_zigzag_does_not_silently_duplicate_coords(self): + # Belt-and-braces: confirm the symptom described in the issue + # (``.sel(x=0.5)`` returning two columns) is impossible now + # because the rasterize call raises before it can produce a + # DataArray with duplicate coords. + x_2566 = np.array([0.5, 1.5, 0.5, 1.5]) + y_2566 = np.linspace(3.5, 0.5, 4) + like_2566 = _make_like(x_2566, y_2566) + with pytest.raises(ValueError): + rasterize( + [(box(0, 0, 2, 4), 1.0)], + like=like_2566, fill=0, + ) From 43fb6ff8e8e6f1ebd17c3d7b2d0c2d89d5667da9 Mon Sep 17 00:00:00 2001 From: Brendan Collins Date: Thu, 28 May 2026 08:29:24 -0700 Subject: [PATCH 2/2] Address review nits: refresh caller comment and test docstring (#2566) - Update the caller-side comment at _extract_grid_from_like that still described the old abs(diff) check, so it now reflects the signed-diff comparison. - Update the test file docstring to list the seventh pinned case (zig-zag symptom check) for parity with the actual test count. Defers the `expected_step` parameter rename / removal to a follow-up, since the parameter is still consulted for the == 0 and isfinite short-circuits and the rename would expand the surgical scope of the bug fix. --- xrspatial/rasterize.py | 11 ++++++----- xrspatial/tests/test_rasterize_signed_step_2566.py | 3 +++ 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/xrspatial/rasterize.py b/xrspatial/rasterize.py index e94c061a8..2d6f78af6 100644 --- a/xrspatial/rasterize.py +++ b/xrspatial/rasterize.py @@ -2922,11 +2922,12 @@ def _extract_grid_from_like(like): # pixel lives. Validate uniform spacing here so the rasterizer never # produces a DataArray whose coords disagree with its data layout. # - # Compare ``abs(diff)`` against the first interval so the check stays - # agnostic to axis direction -- ascending or descending y both pass as - # long as the spacing is uniform. Use ``np.allclose`` rather than - # strict equality because affine-transform-derived coords drift by a - # few ulps. + # Compare *signed* diffs against the signed first interval so the + # check accepts ascending and descending axes (the sign of the + # first interval carries the direction) but rejects zig-zag / + # duplicate-coord patterns whose abs(diff) happens to be uniform. + # Use ``np.allclose`` rather than strict equality because affine- + # transform-derived coords drift by a few ulps. _check_uniform_axis('x', x, px) _check_uniform_axis('y', y, py) diff --git a/xrspatial/tests/test_rasterize_signed_step_2566.py b/xrspatial/tests/test_rasterize_signed_step_2566.py index 91bb892b0..197b82f80 100644 --- a/xrspatial/tests/test_rasterize_signed_step_2566.py +++ b/xrspatial/tests/test_rasterize_signed_step_2566.py @@ -24,6 +24,9 @@ 5. Strictly increasing but non-uniform spacing is still rejected (the original #2168 case). 6. Single-cell axes still short-circuit cleanly. +7. The zig-zag symptom from the issue (``.sel(x=0.5)`` returning two + columns) cannot occur because the call raises before producing a + DataArray with duplicate coords. """ import numpy as np