Skip to content

Drop zero-length contour geometry at exact-level corners#2902

Merged
brendancol merged 5 commits into
mainfrom
issue-2892
Jun 3, 2026
Merged

Drop zero-length contour geometry at exact-level corners#2902
brendancol merged 5 commits into
mainfrom
issue-2892

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #2892

Problem

contours() classifies corners with >= level, so a corner exactly equal to the level is treated as above it. When that happens the marching-squares interpolation lands the crossing on the corner, and both endpoints of a segment can collapse onto the same point. The result is a zero-length segment that becomes an invalid LineString (is_valid == False, length 0) once it reaches the GeoDataFrame.

A 4x4 checkerboard of 0 and 1 at level=1.0 produced 8 such zero-length geometries.

Fix

Filter degenerate geometry in the backend-agnostic stitching/output layer, so both return_type="numpy" and return_type="geopandas" stay clean:

  • _emit_seg skips writing a segment when its two endpoints are identical (zero length).
  • _stitch_segments drops any polyline that collapses to a single distinct point.
  • The policy is documented in the contours() docstring.

Corners are still classified with >= level (equality counts as above), and that rule is applied the same way every time. Genuine mid-edge crossings are untouched.

Backends

The fix lives in shared Python helpers that all four backends route through (_emit_seg, _stitch_segments), so numpy, cupy, dask+numpy, and dask+cupy get the same behavior.

Test plan

  • Checkerboard at level=1.0 produces no zero-length segments and no invalid LineStrings (numpy and dask)
  • No repeated consecutive points in emitted geometries
  • Equality semantics consistent across checkerboard phase variants
  • Genuine ramp crossing still produces valid contours (regression guard)
  • Existing contour tests still pass (53 total, 7 new)

@github-actions github-actions Bot added the performance PR touches performance-sensitive code label Jun 3, 2026

@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: Drop zero-length contour geometry at exact-level corners

Blockers (must fix before merge)

None.

Suggestions (should fix, not blocking)

  • xrspatial/contour.py _to_geopandas (around line 547): the geopandas builder still only guards len(coords) >= 2. After this PR the upstream _stitch_segments filter removes single-point polylines, so nothing degenerate reaches it in practice. But the output layer has no independent guard, so a future change to the stitching path could let a zero-length 2-point line through again. A one-line distinct-point check here would make the geopandas path self-defending. Optional, since the upstream filter covers the current code.

Nits (optional improvements)

  • _emit_seg uses exact float equality (r0 == r1 and c0 == c1). This is correct for the real collapse case, because both endpoints are computed from the same corner values and come out bit-identical. A one-line comment noting that exact equality is intended (near-equal points are genuine short segments) would save the next reader a double-take.

What looks good

  • The fix sits in shared helpers (_emit_seg, _stitch_segments) that all four backends route through, so numpy, cupy, dask+numpy, and dask+cupy get identical behavior without per-backend code.
  • _has_distinct_points reuses the existing DECIMALS=10 rounding tolerance from the stitching logic, so the distinct-point test matches how endpoints are already compared.
  • The tests are hard, not forgiving. They assert no zero-length segments, no invalid LineStrings, and no repeated consecutive points, and they were confirmed to fail without the fix (6 of 7 fail on the pre-fix code; the 7th is the genuine-crossing regression guard).
  • The >= level equality rule is documented in the docstring and kept consistent.

Checklist

  • Algorithm matches reference (marching squares unchanged; only degenerate output filtered)
  • All four backends produce consistent results (shared helpers)
  • NaN handling unchanged (non-finite quads still skipped upstream)
  • Edge cases covered (checkerboard, phase variants, genuine crossing)
  • Dask chunk boundaries handled (filter runs in both per-chunk and re-stitch paths)
  • No premature materialization or extra copies
  • Benchmark not needed (bug fix, no new function)
  • README feature matrix unchanged (no new function, backends unchanged)
  • Docstring updated

@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 (after commit 561fdaa)

Both findings from the first pass are addressed.

Suggestion (resolved)

_to_geopandas now calls _has_distinct_points(coords, 10) instead of the bare len(coords) >= 2 check, so the geopandas path drops zero-length / single-point lines on its own rather than relying only on the upstream stitch filter.

Nit (resolved)

The _emit_seg zero-length guard now carries a comment explaining that exact float equality is intentional: a real collapse yields bit-identical coordinates, while near-equal endpoints are genuine short segments and are kept.

New findings

None. The follow-up is two small, contained edits. All 53 contour tests pass (7 new for this issue).

Blockers

None.

@brendancol brendancol merged commit 6651247 into main Jun 3, 2026
3 of 7 checks passed
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.

contours() emits zero-length, invalid LineStrings when corners equal the contour level

1 participant