Repository navigation
Conversation
echoix
force-pushed
the
claude/raster2numpy-region-timing-fix
branch
from
September 5, 2026 02:59
80fc75c to
c4916ec
Compare
echoix
force-pushed
the
claude/raster-region-windows-followup
branch
from
September 5, 2026 02:59
de713f3 to
800a40f
Compare
echoix
marked this pull request as ready for review
September 5, 2026 05:04
echoix
force-pushed
the
claude/raster2numpy-region-timing-fix
branch
from
September 5, 2026 21:07
c4916ec to
4f13a2d
Compare
echoix
force-pushed
the
claude/raster-region-windows-followup
branch
from
September 5, 2026 21:07
800a40f to
6c37419
Compare
echoix
force-pushed
the
claude/raster2numpy-region-timing-fix
branch
2 times, most recently
from
September 5, 2026 21:09
fee78ab to
0a385ac
Compare
echoix
force-pushed
the
claude/raster-region-windows-followup
branch
2 times, most recently
from
September 5, 2026 21:12
12843ef to
0c30f10
Compare
echoix
force-pushed
the
claude/raster2numpy-region-timing-fix
branch
from
September 6, 2026 11:04
0a385ac to
4a7dc34
Compare
echoix
force-pushed
the
claude/raster-region-windows-followup
branch
from
September 6, 2026 11:04
0c30f10 to
a8b514d
Compare
Follow-up to OSGeo#7902: fixing the resource leak there turned a fatal, whole-file-crashing FERROR into three individually diagnosable failures instead. The remaining issue is real but distinct from the leak: the small region these tests set up isn't actually restricting what RasterRow reads back (rast[0] comes back with roughly 100000 elements instead of 10). Built GRASS locally and confirmed this is genuinely Windows-specific, not a state leak reproducible here: the file passes reliably on Linux both run in isolation and as part of the full 359-file gunittest suite in sequence. Marking xfail on Windows per the existing convention rather than guessing at a fix that can't be verified; needs investigation with actual Windows access.
echoix
changed the base branch from
claude/raster2numpy-region-timing-fix
to
main
September 6, 2026 11:22
echoix
force-pushed
the
claude/raster-region-windows-followup
branch
from
September 6, 2026 11:22
a8b514d to
add9cd3
Compare
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.
Description
Follow-up to OSGeo#7902 (
grass.pygrass: Close RasterRow via context manager in region tests, already merged to main). Fixing that resource leak turned the fatal, whole-file-crashingFERROR: Input window changed while maps are open for readon Windows CI into a distinct, real bug: the small region thattest_resampling_1,test_resampling_2, andtest_resampling_to_numpyeach set up isn't actually restricting whatRasterRow/raster2numpyread back. Windows CI showed, for example,rast[0].tolist()coming back with roughly 100000 elements instead of the expected 10, and a numpy_ArrayMemoryErrortrying to allocate an 8,150,000-element array where an 8-element one was expected.I built GRASS locally (CMake) to check whether this is a genuine Windows-only build difference or something reproducible here:
test_raster_region.pyrun in isolation: passes (3/3).Since it doesn't reproduce here even under matching full-suite conditions, this looks like a genuine platform difference in the compiled GRASS libraries (Windows CI here builds with MinGW via OSGeo4W) rather than a Python-level state leak, and isn't something I can responsibly guess a fix for without Windows access. Marked
@xfail_windowson the three affected test methods instead, per the project's existing convention for exactly this situation (seepython/grass/gunittest/utils.py), with a comment describing the symptom and noting it needs Windows access to debug further.Motivation and context
OSGeo#7902 already turned the fatal crash into three individually visible failures. Without
xfail, those would show as ordinary CI red without acknowledging they're a known, unreproduced, Windows-only issue distinct from the leak OSGeo#7902 fixed.How has this been tested?
Built GRASS locally and ran the test both in isolation and as part of the full local gunittest suite (see above); confirmed it still passes on Linux with
@xfail_windowsadded (a no-op there).Types of changes
Checklist
pre-defined prefixes
of this project.
I used Claude Code to help investigate and draft this fix, under my review throughout. OSGeo#7902 already fixed the crash that was masking this bug; this PR doesn't fix the bug itself, only marks it
xfailon Windows using the project's own existing mechanism for an unreproduced platform-specific failure, rather than claiming a fix I can't verify.