Skip to content

Add DimensionalData extension for polygonize#429

Merged
asinghvi17 merged 5 commits into
mainfrom
as/dimensionaldata-ext
Jul 5, 2026
Merged

Add DimensionalData extension for polygonize#429
asinghvi17 merged 5 commits into
mainfrom
as/dimensionaldata-ext

Conversation

@asinghvi17

@asinghvi17 asinghvi17 commented Jul 4, 2026

Copy link
Copy Markdown
Member

Freshens and rebases #178 (originally by @rafaqz) onto current main, adds a DimensionalData extension for polygonize, and wires the polygonize tests into CI (fixing the bugs that surfaced).

What it does

Adds GeometryOpsDimensionalDataExt, which polygonizes any AbstractDimArray (e.g. a DimArray or Raster) using its X/Y lookup coordinate values (via interval bounds) rather than the raw integer axes. Points sampling is treated as Intervals (with a warning); the array's crs is preserved.

polygonize now runs in CI

test/methods/polygonize.jl was never registered in runtests.jl (on main too), so polygonize had no CI coverage. It's now wired in. Getting it green required fixing several pre-existing issues:

Real source bugs in _pixel_edges (offset-array handling):

  1. It indexed its 1-based pixel-bound vectors (xs/ys) with the array's native axes → BoundsError on any OffsetArray. Now indexes A by native axes and the bound vectors positionally.
  2. The S/W boundary guards compared the row index against the first/last column index (and vice versa). Harmless for 1-based or equal-offset arrays, but a BoundsError for arrays whose two axes start at different offsets (e.g. Origin(-100, -50)) with edge pixels. Fixed by naming the boundary indices by row/column. No-op for 1-based arrays.

Test-harness fixes:

  • @test_nowarn multipolygon = polygonize(A) no longer leaks the binding on Julia 1.12 → changed to multipolygon = @test_nowarn polygonize(A).
  • The file now seeds the RNG for determinism.
  • The DimensionalData/Rasters subtests use offset, non-square lookups so they actually exercise the extension's coordinate-awareness (against integer axes they'd pass trivially).

Fixes to the original #178

  • The extension passed the type DD.AbstractDimArray as polygonize's data argument — a MethodError. Now passes the array A.
  • The Rasters test referenced an undefined evil in its crs assertion.

Known pre-existing bug (NOT fixed here) — flagged via @test_broken

polygonize's ring tracing is coordinate-magnitude-dependent: the same boolean pattern produces a different polygon decomposition depending on the coordinate values (e.g. polygonize(1:100, …) vs polygonize(1001:1100, …) give different polygon counts; simple shapes get a redundant collinear vertex at a coordinate-hash-dependent start point; some coordinate ranges even hit a KeyError). Two assertions that assume translation/scale invariance are marked @test_broken with comments pointing here. This is a separate ring-tracing bug worth its own issue/PR.

Verification

Full test suite passes (Pkg.test(), exit 0). The Polygonize testset runs in-suite: 74 pass / 2 broken (the documented @test_broken), 0 fail/error.

Closes #178.

🤖 Generated with Claude Code

rafaqz and others added 3 commits July 4, 2026 14:31
Adds `GeometryOpsDimensionalDataExt`, which polygonizes any
`AbstractDimArray` (e.g. a `DimArray` or `Raster`) using its `X`/`Y` lookup
values via interval bounds, rather than the raw integer axes. Falls back to
treating `Points` sampling as `Intervals` (with a warning), and preserves the
array's crs.

Rebased onto current `main`: drops the now-redundant Julia 1.9->1.10 bump, and
adapts `Project.toml` to the current `[weakdeps]`/`[extensions]`/`[compat]`
layout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`_pixel_edges` indexed its 1-based pixel-bound vectors (`xs`/`ys`) with the
array's native axes, throwing a `BoundsError` for arrays with offset axes
(e.g. `OffsetArray`). Index `A` by its native axes but the bound vectors
positionally instead.

Register `test/methods/polygonize.jl` in `runtests.jl` (it was never wired
in, so `polygonize` — and the new DimensionalData extension — had no CI
coverage). Make the file deterministic (seed the RNG) and fix a
`@test_nowarn multipolygon = polygonize(A)` binding-scope error on Julia 1.12.

Two assertions are marked `@test_broken`: `polygonize`'s ring tracing is
coordinate-magnitude-dependent, so rescaling/translating the coordinates
changes the decomposition. That is a separate pre-existing bug.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`_pixel_edges` compared the row index against the first/last *column* index
(and vice versa) in the S/W neighbour guards. When an array's two axes start
at different offsets those guards no longer short-circuit at the boundary, so
polygonizing e.g. `OffsetArrays.Origin(-100, -50)(...)` with edge pixels threw
a `BoundsError`. Name the boundary indices by row/column so each guard matches
its axis. No-op for 1-based (and equal-offset) arrays.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
asinghvi17 and others added 2 commits July 4, 2026 23:26
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@asinghvi17 asinghvi17 merged commit 95c9753 into main Jul 5, 2026
9 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.

2 participants