Add viewshed tests for 1x1, all-NaN, and Inf-terrain edge cases#3426
Open
brendancol wants to merge 1 commit into
Open
Add viewshed tests for 1x1, all-NaN, and Inf-terrain edge cases#3426brendancol wants to merge 1 commit into
brendancol wants to merge 1 commit into
Conversation
brendancol
commented
Jun 20, 2026
brendancol
left a comment
Contributor
Author
There was a problem hiding this comment.
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_inputhardcodes the observer's flat index as2 * 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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3425
Test-only change from the test-coverage sweep. Three input shapes that nothing in
test_viewshed.pyexercised, all already behaving correctly:test_viewshed_single_pixel: a 1x1 raster returns[[180.0]](the degenerate-axis resolution guard from viewshed() returns wrong results for single-row or single-column rasters (divide-by-zero in resolution) #2744 keeps it from dividing by zero)test_viewshed_inf_terrain_cell: aninfelevation cell comes back visible and doesn't leak NaN into the rest of the gridtest_viewshed_all_nan_input: an all-NaN raster gives observer=180 and INVISIBLE everywhere elseEach 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)