Skip to content

rasterize/kde/line_density: set output attrs['res'] when grid resolution changes (#3571)#3572

Merged
brendancol merged 2 commits into
mainfrom
issue-3571
Jun 29, 2026
Merged

rasterize/kde/line_density: set output attrs['res'] when grid resolution changes (#3571)#3572
brendancol merged 2 commits into
mainfrom
issue-3571

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3571

Tools that build an output grid at a different resolution than their input now write the correct res into the output attrs. get_dataarray_resolution() reads attrs['res'] before deriving cell size from coordinates, so a missing or stale res was handing the wrong cellsize to downstream distance/area ops like slope, aspect, and proximity.

  • rasterize(): the reshape path (caller overrides bounds/width/height/resolution, or rasterizes with no like at all) now records the freshly-built grid's cell size as attrs['res'] instead of leaving it absent. The reuse-like-coords path still keeps the template's res/transform unchanged. A stale inherited transform is still dropped on reshape.
  • kde() / line_density(): when template is omitted, the output now carries attrs['res'] = (abs(dx), abs(dy)) for the grid built from x_range/y_range.

resample() already did this on every path; this aligns the remaining resolution-changing tools with it. preview() has the same class of issue and is handled in a separate PR.

Backend coverage: the rasterize change covers numpy, cupy, dask+numpy, and dask+cupy (attrs are built backend-independently, and the #2251 test class exercises all four). The kde/line_density change is on the eager template=None construction path.

Test plan:

  • Updated the issue rasterize: stale transform/res attrs propagated from like when grid is overridden #2251 rasterize tests (and the no-like rectangular-pixel test) to assert the refreshed res value instead of asserting res is absent.
  • Added TestKDEResolutionAttr covering kde/line_density res from range, res-matches-coords, and template-supplied res.
  • test_kde.py, test_rasterize.py, and test_rasterize_coverage_2026_05_21.py pass (340 passed, 2 skipped).

…3571)

rasterize(): the reshape path (caller overrides bounds/width/height/
resolution, or rasterizes without like=) now records the freshly-built
grid's cell size in attrs['res'] instead of leaving it absent. The
reuse-like-coords path still keeps the template's res/transform. Stale
inherited transform is still dropped on reshape.

kde()/line_density(): when template is omitted, set attrs['res'] to the
(dx, dy) cell spacing of the grid built from x_range/y_range.

get_dataarray_resolution() prefers attrs['res'] over deriving cellsize
from coords, so a missing or stale res fed the wrong cellsize to
downstream slope/aspect/proximity callers. resample() already did this;
this brings rasterize/kde/line_density in line.

Updates the issue #2251 tests to assert the refreshed res value rather
than its absence.

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: rasterize/kde/line_density: set output attrs['res'] when grid resolution changes (#3571)

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

None.

Nits (optional improvements)

  • xrspatial/kde.py:676, 805 — For a single-column or single-row output (width=1 or height=1), dx/dy comes out as span / max(cols-1, 1) = span, so res reports the full extent on that axis (e.g. width=1 over x_range=(0,10) gives res_x=10 against a single x coord at 0). That's a degenerate grid and the value just echoes the existing dx/dy binning, so it isn't wrong, just meaningless. Not worth special-casing unless single-pixel KDE grids are a real use case.
  • test_kde.pytest_kde_res_matches_coords checks res-vs-coords and get_dataarray_resolution for kde; line_density only gets the from-range assertion. A matching res-vs-coords check for line_density would be symmetric, even though the construction path is the same.

What looks good

  • The reuse-like-coords path is untouched, so the bit-identical-template contract and the existing res/transform preservation still hold. Only the genuinely-reshaped path changes.
  • abs() on px/py/dx/dy keeps res positive when a caller passes a descending x_range/y_range (checked: x_range=(10,0) yields res (1.0, 1.0), not negative).
  • res matches the coordinate-derived cellsize under both conventions: rasterize uses cell-center coords, kde uses an endpoint-inclusive linspace, and res equals the coord spacing in each.
  • Stale transform is still dropped on reshape, so res and transform can't disagree.
  • The #2251 test class was updated rather than deleted, so the regression coverage survives and now pins the stronger contract: res is present and correct instead of just absent.

Checklist

  • res order is (x_res, y_res), consistent with resample() and get_dataarray_resolution()
  • All four backends covered for rasterize (attrs built post-dispatch, backend-independent; #2251 dask/cupy tests assert res)
  • NaN handling unaffected (metadata-only change)
  • Edge cases covered (rectangular pixels, no-like, like-reshape, template-supplied)
  • Dask chunk boundaries unaffected (no compute path change)
  • No premature materialization or copies introduced
  • No benchmark needed (metadata-only)
  • README feature matrix not applicable (no new function, no backend change)
  • Docstrings updated for kde, line_density, and rasterize Returns

@brendancol brendancol left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review (#3571)

Re-reviewed after the follow-up commit (939d358a). The commit only adds a test, no production change, so the prior review's findings still stand. Disposition of the two nits:

  • Nit 2 (line_density res-vs-coords test) — fixed in 939d358a via test_line_density_res_matches_coords, mirroring the kde check.
  • Nit 1 (single-cell res reports the full extent) — dismissed. For a 1-cell axis, res mirrors the dx/dy the kernel already uses for binning, so it stays consistent with how the grid was built. There's no sensible alternative value, and special-casing a degenerate config would add code for no real gain.

No blockers or suggestions. Good to merge once CI is green.

@brendancol brendancol merged commit 8a31598 into main Jun 29, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rasterize/kde/line_density: output attrs['res'] not updated when output grid resolution differs from input

1 participant