Skip to content

r.fill.stats: migrate tests from gunittest to pytest - #7894

Open
Valyrian-Code wants to merge 3 commits into
OSGeo:mainfrom
Valyrian-Code:r.fill.stats-pytest
Open

Valyrian-Code wants to merge 3 commits into
OSGeo:mainfrom
Valyrian-Code:r.fill.stats-pytest

Conversation

@Valyrian-Code

@Valyrian-Code Valyrian-Code commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Migrates the r.fill.stats testsuite from gunittest to pytest, continuing the one-at-a-time migrations from #7635/#7675/#7780/#7893.

Same input and reference outputs as before, now inlined and compared cell by cell with numpy instead of through a difference raster.

Found one discrepancy while doing that: the reference data has the northeast corner cell as null in all four modes, but the tool currently fills it in all four, with a different value per mode (as expected, since each mode computes a different statistic). Every other cell matches the old reference exactly, to float64 precision.

The old test's assertRastersNoDifference could not have caught this: it checks the range of a difference raster, and a difference against a null cell is itself null, so it gets excluded from that range check.

No relevant change to the fill algorithm since this test was added in 2013 explains the corner behavior, so I believe the original reference was wrong for that one cell. This test asserts the corner's current value explicitly rather than reusing the old expectation. Happy to open a separate issue if that seems worthwhile.

Verified locally, all four cases pass.

@github-actions github-actions Bot added raster Related to raster data processing Python Related code is in Python module tests Related to Test Suite labels Sep 5, 2026
@Valyrian-Code

Copy link
Copy Markdown
Contributor Author

Same CI infra issue as noted on #7780: the failing checks are hitting an HTTP 504 from Launchpad while adding the ubuntugis-unstable PPA, unrelated to this PR. Should clear with a re-run.

@Valyrian-Code
Valyrian-Code marked this pull request as ready for review September 19, 2026 18:40
Copilot AI lite review requested due to automatic review settings September 19, 2026 18:40

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The NumPy comparison retains a relative tolerance, weakening the previous absolute 1e-6 assertion.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Medium severity

Open (1)
What changed in this PR

Migrates r.fill.stats regression coverage from gunittest to pytest, using isolated GRASS sessions, grass.tools, and NumPy comparisons.

Changes:

  • Adds parametrized tests for all four interpolation modes.
  • Inlines input and reference raster data.
  • Removes the legacy gunittest test and data files.
File Description
raster/​r.fill.stats/​tests/​r_fill_stats_test.py New pytest-based regression tests and inline fixtures.
raster/​r.fill.stats/​testsuite/​test_r_fill_stats.py Removes the legacy gunittest test.
raster/​r.fill.stats/​testsuite/​data/​input_ascii.txt Removes migrated input data.
raster/​r.fill.stats/​testsuite/​data/​output_wmean.txt Removes migrated weighted-mean reference.
raster/​r.fill.stats/​testsuite/​data/​output_mean.txt Removes migrated mean reference.
raster/​r.fill.stats/​testsuite/​data/​output_median.txt Removes migrated median reference.
raster/​r.fill.stats/​testsuite/​data/​output_mode.txt Removes migrated mode reference.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread raster/r.fill.stats/tests/r_fill_stats_test.py Outdated
np.allclose applies rtol as well as atol, so with the default rtol=1e-5 a
cell near 5 could differ by about 5e-5 and still pass, where the gunittest
version used an absolute precision=1e-6. Setting rtol=0 restores that.
@Valyrian-Code

Copy link
Copy Markdown
Contributor Author

Good catch, fixed. The original was an absolute precision=1e-6, and the default rtol=1e-5 let a cell near 5 drift about 5e-5, so this had quietly loosened it. Set rtol=0, all four modes still pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module Python Related code is in Python raster Related to raster data processing tests Related to Test Suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants