Skip to content

Add viewshed tests for 1x1, all-NaN, and Inf-terrain edge cases#3426

Open
brendancol wants to merge 1 commit into
mainfrom
deep-sweep-test-coverage-viewshed-2026-06-20-01
Open

Add viewshed tests for 1x1, all-NaN, and Inf-terrain edge cases#3426
brendancol wants to merge 1 commit into
mainfrom
deep-sweep-test-coverage-viewshed-2026-06-20-01

Conversation

@brendancol

Copy link
Copy Markdown
Contributor

Closes #3425

Test-only change from the test-coverage sweep. Three input shapes that nothing in test_viewshed.py exercised, all already behaving correctly:

Each is parametrized over numpy and dask Tier B (the CPU-backed backends). The cupy/RTX path is a separate implementation and is out of scope here.

No source change. I probed all three by hand first to confirm the behavior was already correct, so these pin existing behavior rather than fixing a bug.

Test plan:

  • pytest xrspatial/tests/test_viewshed.py — 120 passed, 1 skipped (no-RTX fallback)
  • the 6 new test cases run and pass on a CUDA+RTX host
  • flake8 clean on the test file

@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: viewshed edge-case test coverage

Test-only change, three new tests parametrized over numpy and dask Tier B. I read all three in full and ran the suite. No blockers, no suggestions.

Blockers

None.

Suggestions

None.

Nits

  • test_viewshed_all_nan_input hardcodes the observer's flat index as 2 * 5 + 2. It's tied to the fixed 5x5 grid and the inline comment explains it, so this is fine as-is; just noting it for anyone who later changes the grid size.

What looks good

  • Each test checks against known values (observer cell = 180, INVISIBLE elsewhere), not just "runs without crashing".
  • NaN checks use np.isnan, and the inf test explicitly asserts no NaN leaks into the rest of the grid.
  • Backend scope (numpy + dask Tier B, cupy/RTX out) matches the existing NaN tests in this file and is called out in each docstring.
  • 1x1 dask uses a (1,1) chunk, which exercises the degenerate-axis resolution guard on the dask path too.

Checklist

  • Algorithm matches reference/paper (n/a — test-only)
  • All implemented backends produce consistent results (numpy + dask asserted; cupy/RTX scoped out)
  • NaN handling is correct
  • Edge cases are covered by tests (that's the PR)
  • Dask chunk boundaries handled correctly
  • No premature materialization or unnecessary copies
  • Benchmark exists or is not needed (not needed — tests)
  • README feature matrix updated (n/a)
  • Docstrings present and accurate

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.

viewshed: add test coverage for 1x1, all-NaN, and Inf-terrain edge cases

1 participant