Skip to content

Decouple datashader from xarray-spatial#3641

Open
Melissari1997 wants to merge 8 commits into
xarray-contrib:mainfrom
Melissari1997:issue-3640
Open

Decouple datashader from xarray-spatial#3641
Melissari1997 wants to merge 8 commits into
xarray-contrib:mainfrom
Melissari1997:issue-3640

Conversation

@Melissari1997

Copy link
Copy Markdown
Collaborator

Closes #3640. Implements the consolidated datashader decoupling across #3450, #3451, and #3453#3464 (#3452 was already closed).

What changed

Backend coverage

No backend code changed — this is a packaging/docs/internal-cleanup effort with no change to any public xrspatial.* function. numpy / cupy / dask+numpy / dask+cupy paths are untouched.

Test plan

  • test_canvas_like_removed_3451_3640 regression test added — asserts canvas_like stays absent and import xrspatial.utils does not load datashader.
  • pytest xrspatial/tests/test_utils.py test_terrain.py test_zonal.py -k "not dask" — 139 passed, 42 skipped (dask backends skipped because dask is not installed in this env; unrelated to the change).
  • xrspatial/ source has zero datashader imports (only an explanatory docstring comment in zonal.py).
  • All 11 targeted notebooks are valid JSON with zero datashader imports; animated_hillshade.py compiles.
  • Notebook output cells were cleared (matplotlib/nbconvert could not be installed in this env to re-execute). Source cells are run-ready; CI or a maintainer can re-execute to regenerate preview images.

Deliberate datashader references kept

  • The A/B benchmark in benchmarks/rasterizer_benchmarks.py (import-guarded via HAS_DATASHADER).
  • Historical-credit prose in docs/source/getting_started/why_xarray_spatial.rst and docs/source/index.rst.

Notes

  • CHANGELOG.md was not touched (per rockout rules; maintained separately at release time).
  • No new public API; no behavior changes.

…, xarray-contrib#3640)

- Delete canvas_like() from xrspatial/utils.py (incl. the import
  datashader as ds try/except). canvas_like had no callers in xrspatial/
  and was never exported.
- Rewrite the suggest_zonal_canvas docstring example in xrspatial/zonal.py
  to use the arr.xrs.rasterize(...) accessor instead of ds.Canvas().polygons().
- Reword the pixel-center comment in xrspatial/terrain.py so it no longer
  names datashader.

No public API change. Library source now has zero datashader imports.
…xarray-contrib#3459, xarray-contrib#3463, xarray-contrib#3464, xarray-contrib#3640)

- fire.ipynb: shade(...) -> plt.imshow with LinearSegmentedColormap;
  pre/post NBR Images(...) -> side-by-side plt.subplots.
- classification-methods example: drop datashader import; shade(...)
  with plt.get_cmap('terrain') -> plt.imshow(..., cmap='terrain').
- animated_hillshade.py: rewrite to use matplotlib colormaps + PIL
  alpha compositing instead of datashader shade/stack; build the
  terrain template as a plain xarray.DataArray (generate_terrain no
  longer takes a canvas= argument).

Outputs cleared; source has zero datashader imports.
…ader (xarray-contrib#3453, xarray-contrib#3454, xarray-contrib#3455, xarray-contrib#3457, xarray-contrib#3640)

- Add a shared _shade/_stack/_show/Elevation helper cell to each notebook;
  _shade maps an array through a matplotlib colormap to RGBA, _stack
  alpha-composites layers, _show displays (with optional background).
- focal: shade/stack of terrain + hillshade composites -> helpers.
- proximity: ds.Canvas().points/line -> .xrs.rasterize on a GeoDataFrame
  of Point/LineString geometries (merge='min' for ds.min); dynspread/
  set_background -> _show(..., bg='black').
- classification: shade/stack -> helpers; Set1/Set3 -> matplotlib cmaps.
- surface: Canvas grid setup -> generate_terrain on a template DataArray;
  the 2D-normal density grid -> np.histogram2d; the single observer point
  -> .xrs.rasterize on a Point GeoDataFrame; shade/stack/dynspread ->
  helpers.

Outputs cleared; source has zero datashader imports.
…atashader (xarray-contrib#3456, xarray-contrib#3458, xarray-contrib#3460, xarray-contrib#3461, xarray-contrib#3462, xarray-contrib#3640)

- zonal: ds.Canvas().line(..., agg=ds.sum('trail_segement_id')) ->
  .xrs.rasterize(line_gdf, column='trail_segement_id', merge='sum');
  shade/stack -> helpers; rewrite 'Datashader aggregate' prose.
- pathfinding: ds.Canvas().line/points -> .xrs.rasterize on Point /
  MultiLineString GeoDataFrames (split NaN-separated segments);
  dynspread/set_background -> _show(..., bg='black').
- multispectral: Canvas().raster(layer, agg='mean') resampling ->
  xarray .coarsen/.interp; shade/stack/Images/orient_array ->
  _shade/_stack/_show/_show_grid helpers.
- Pathfinding_Austin example: ds.Canvas().line/points ->
  .xrs.rasterize on the geopandas GeoDataFrame (drop spatialpandas).
- pharmacy-deserts: ds.Canvas().polygons/points with ds.mean/ds.max ->
  .xrs.rasterize(column=, merge=). merge='mean' is not a supported
  reducer, so non-overlapping census polygons use merge='last'
  (equivalent for a single covering polygon per pixel); counties use
  merge='max' to preserve the ds.max intent. inferno/Set1 -> matplotlib.

Outputs cleared; source has zero datashader imports.
…array-contrib#3640)

- setup.cfg: replace 'datashader >= 0.15.0' in the examples extra with
  matplotlib, geopandas, and shapely (the rendering and vector-
  rasterization deps the example notebooks now use). datashader is no
  longer pulled by 'pip install xarray-spatial[examples]'.
- installation.rst: update the examples extra row to list matplotlib,
  geopandas, shapely; note datashader is no longer required.

Deliberate datashader references kept: the A/B benchmark in
benchmarks/rasterizer_benchmarks.py (import-guarded) and the historical-
credit prose in why_xarray_spatial.rst / index.rst.

Library source has zero datashader imports. Closes the consolidated
decoupling effort for xarray-contrib#3451, xarray-contrib#3453-xarray-contrib#3464.
…ray-contrib#3640)

Locks in that canvas_like() stays removed and that importing
xrspatial.utils does not load datashader, guarding against accidental
re-introduction of the library-source datashader dependency.
@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jul 5, 2026
@Melissari1997 Melissari1997 changed the title Decouple datashader from xarray-spatial (#3450, #3451, #3453-#3464) Decouple datashader from xarray-spatial Jul 5, 2026

@Melissari1997 Melissari1997 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

PR Review: Decouple datashader from xarray-spatial

Reviewed the diff against main (18 files). The change removes datashader from library source, all 9 user-guide notebooks, 3 example notebooks/script, and the examples packaging extra, with no public API changes. The approach (substitute template.xrs.rasterize(...) for vector rasterization, matplotlib _shade/_stack/_show helpers for rendering, xarray .coarsen/.interp for raster resampling) is sound and matches the consolidated plan.

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

  • examples/animated_hillshade.py:115-124_composite has a dead alpha branch. Line 120 reassigns base to an RGB array (top[..., :3] * a + base[..., :3] * (1 - a), shape (H, W, 3)), so the guard on line 121 (base.shape[-1] == 4) is always false and the output alpha is hard-coded to 255. The visual output happens to look right because the terrain/water base layers are fully opaque, but the compositing is incorrect for any layer stack with a transparent base. The notebook _stack helper (e.g. focal.ipynb, classification.ipynb) does this correctly by keeping rgb separate and rebuilding base with np.concatenate([rgb, out_a[..., None]], axis=-1). Recommend aligning _composite with that implementation so the alpha math is right and the dead branch goes away.

  • examples/pharmacy-deserts.ipynb cell that rasterizes block groups — the comment says merge='mean' is "equivalent to datashader's ds.mean('PCT_ABOVE_65') here" because block groups tile the plane. That equivalence holds only if polygons do not overlap and each pixel is covered by exactly one polygon. Census block groups are designed to tile, so this is correct in practice, but worth a one-line note that the equivalence relies on non-overlap (the current comment is close but could state the assumption more sharply). No code change needed if the comment is tightened.

Nits (optional improvements)

  • xrspatial/zonal.py docstring example — ys = np.linspace(y_range[0], y_range[1], height) uses ascending y. The original ds.Canvas y-orientation differs, but the actual_min_pixels count assertion is invariant to a y-flip so the doctest still passes if it were run. No action required; flagging only so a future reader doesn't assume the y-direction was carefully matched.

  • The _shade helper is duplicated verbatim across 8 notebooks. This is acceptable for self-contained notebooks, but if a shared notebook helper module ever exists, these would be good candidates to centralize. Not blocking.

  • examples/animated_hillshade.py:7 docstring says "Requires: numpy, xarray, matplotlib, pillow." — also needs xrspatial (imported on line 16). Trivial docstring fix.

What looks good

  • canvas_like() removal is clean — it had no callers in xrspatial/ and was never exported, and the regression test (test_canvas_like_removed_3451_3640) guards against re-introduction, including checking that import xrspatial.utils does not load datashader via sys.modules.
  • The packaging change correctly adds matplotlib, geopandas, shapely to the examples extra (the deps the migrated notebooks actually use) rather than just dropping datashader.
  • Deliberate datashader references (the A/B benchmark in benchmarks/rasterizer_benchmarks.py and the historical-credit prose) were correctly preserved.
  • The merge= mapping for the multi-reducer cases is correct: ds.minmerge='min', ds.summerge='sum', ds.maxmerge='max'. The ds.mean case is correctly flagged as unsupported and handled with merge='last' on non-overlapping polygons.
  • All 11 notebooks are valid JSON with zero datashader imports; all Python files compile.

Checklist

  • N/A — no algorithm changes (packaging/docs/internal cleanup only)
  • N/A — no backend code changed; numpy/cupy/dask paths untouched
  • N/A — no neighborhood operations changed
  • Regression test added for the one source removal (canvas_like)
  • N/A — no dask chunk changes
  • No premature materialization introduced (notebook helpers operate on numpy arrays)
  • N/A — no new benchmarkable function
  • N/A — no new public function (README matrix unchanged, correctly)
  • Docstring rewritten for suggest_zonal_canvas; comment reworded in terrain.py

…y-contrib#3640)

- animated_hillshade.py: _composite had a dead alpha branch (line 120
  reassigned base to RGB, so the base.shape[-1]==4 guard on line 121
  was always false and output alpha was hard-coded to 255). Rewrite to
  keep rgb separate and rebuild base with np.concatenate, matching the
  _stack helper used in the notebooks. Also add xrspatial to the
  requires line in the docstring.
- pharmacy-deserts.ipynb: sharpen the merge='last' comment to state the
  non-overlap assumption (each pixel covered by exactly one polygon)
  that makes it equivalent to ds.mean.

@Melissari1997 Melissari1997 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Follow-up review (after review-driven fixes)

Commit c2a85c85 addresses the two Suggestions and the actionable Nit from the first review.

Disposition of original findings

  • Suggestion 1 — animated_hillshade.py _composite dead alpha branch: fixed. Rewrote _composite to keep rgb separate and rebuild base with np.concatenate([rgb, out_a], axis=-1), matching the notebook _stack helper. The output alpha is now tracked correctly through the layer stack instead of being hard-coded to 255.

  • Suggestion 2 — pharmacy-deserts.ipynb merge='last' comment: fixed. Sharpened the comment to state the non-overlap assumption ("each pixel is covered by exactly one polygon") that makes merge='last' equivalent to ds.mean for the census block groups.

  • Nit — animated_hillshade.py docstring Requires: line: fixed. Added xrspatial to the requires list.

  • Nit — duplicated _shade helper across 8 notebooks: dismissed. Self-contained notebooks are the project convention (each user-guide notebook runs standalone without importing a shared helper module). Centralizing would require a new support module shipped with the docs, which is out of scope for this decoupling effort.

  • Nit — ys linspace direction in the zonal.py docstring: dismissed. The actual_min_pixels count assertion is invariant to a y-flip, and the accessor handles orientation internally. No correctness impact.

What looks good now

  • All review findings are either fixed or dismissed with a recorded reason.
  • animated_hillshade.py compiles; pharmacy-deserts.ipynb is valid JSON.
  • No new findings from the follow-up diff.

@Melissari1997 Melissari1997 self-assigned this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance PR touches performance-sensitive code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decouple datashader from xarray-spatial (consolidated: #3450 + #3451 + #3453-#3464)

1 participant