[SyedAhad01]

Fixes PicklingError failures when running tests with Python 3.14.

Python 3.14 cannot pickle local functions defined inside other functions
when using multiprocessing. This fix moves all run_grid_module inner
functions to module-level helper functions and uses functools.partial
to pass parameters.

Related to #6104

[echoix]

Read our contribution guide, you'll see that most of the back and forth of precious shared CI time can be avoided by using precommit (or prek) to have most simple formatting checks already made.

And I assume you're using an AI or agent or something like that, and I saw you were in VS Code in another PR, so you can reread and stage each line to make sure you're only committing what is relevant to the PR. Touching unrelated lines only makes reviewing harder: the last PR together, it started with many many changes, whilst it ended up being less than 10 lines, two parts. Way easier when concise like that.

[SyedAhad01]

Thank you for the feedback and the guidance. I will read the contribution guide and set up pre-commit before pushing further changes. I will also make sure to carefully review each line and only commit what is directly relevant to the fix

[echoix]

Don't worry, your general approach is fine. There's already some examples in the repo of the pattern needed, that I've fixed as a proof of concept a while ago. And it looked a bit like that.

[echoix]

You don’t have to keep your branch (and draft) updated with main, after each of our commits. We can’t have all hundreds of PRs redo all CI after each commit, it would be exponential

[echoix]

Unless you’re pushing new changes and it will run again, then ya, it’s a good time to make it run on updated code and linters

[SyedAhad01]

Thank you for clarifying. I understand now I should only update the branch when I am pushing my own new changes, not after every commit to main. I will keep that in mind for future PRs. Sorry for the unnecessary CI runs.

[SyedAhad01]

Hi @echoix just a gentle heads-up that the PR is ready for review whenever you have a moment. All 26 checks are passing. I appreciate your time and continued guidance throughout this process.

[echoix]

I'm not sure it is working as you'd think yet. If you look at pytest test in macOS or Windows (Linux should work fine), let's take Windows, the tests of pygrass grid tests should not end up with expected failure anymore. At one point in your development, assuming everything went right, the tests should've been fixed and passing, and they would fail because of an unexpected success. That would have been because the custom attribute @xfail_mp_spawn that wasn't removed.

I see that that attribute's error has changed, but the tests that should've been fixed doesn't have that attribute removed. So the tests still fail, but we ignore it if the default start method is spawn, like Windows.

So, there's still a bit of work to do.

[echoix]

To help you out, I identified the lines where the attributes should be removed, plus other second reading comments

[SyedAhad01]

After removing xfail_mp_spawn, all tests now fail with RuntimeError: Subprocess failed with exit code 1 on both macOS and Windows. It seems the issue is inside GridModule itself where the internal worker functions are not picklable under spawn. Could you point me to the example in the repo that shows the correct pattern for fixing this? I want to make sure I apply the right fix

[SyedAhad01]

I investigated the failures further and it looks like the issue is not in the test wrapper but inside GridModule itself, likely due to multiprocessing spawn requiring top-level worker functions.

I’ll explore the GridModule implementation to identify non-picklable functions and propose a fix.

Please let me know if there is an existing pattern in the repo I should follow.

[echoix]

For the tests, it’s about right. For the other part, I’m not sure there’s a pattern yet, and that is the core of the work, to investigate what’s happening under the hood. Debugging subprocesses is maybe a bit harder, but you probably did a little bit.

It will be easier for you if you manage to do that locally. That’s not the kind of task that you can do by iterating in a PR.

[SyedAhad01]

Thank you for the feedback. I understand now I should debug locally using spawn mode instead of iterating in the PR. I will reproduce the issue locally on Windows with multiprocessing.set_start_method('spawn', force=True), find the root cause, and then submit a clean fix. I will convert back to "Ready for review" once the local debugging is done.

[echoix]

I just had an idea. In some python code recently-ish (last two years), we ended up needing to pass around env/session around in order to be able to properly initialize c-based tools. Is it possible that one of the functions here need to have that too? Like the failures would’ve been because the environment variables are not defined in the new subrosse

[SyedAhad01]

Thank you for the suggestion! I implemented the fix by passing the environment variables explicitly through the work tuple to cmd_exe. However, the tests are still failing with the same RuntimeError: Subprocess failed with exit code 1 on both macOS and Windows. I am continuing to investigate the issue locally to find the root cause.

[SyedAhad01]

Hi @echoix,
I’ve fixed the Python 3.14 compatibility issues and updated the workflow. All CI checks are passing now.

Requesting your review — please let me know if any further changes are needed.

Thanks!

[echoix]

Ill try to read more attentively later

[SyedAhad01]

Done, reverted fail-fast to true. Thank you for the feedback.

[SyedAhad01]

Hi @echoix,
I’ve addressed the previous feedback and all checks are passing now.
Please let me know if anything else needs to be changed.
Thanks.

[SyedAhad01]

Hi @echoix, I hope you’re doing well. Just a gentle follow-up on this PR whenever you have time. Please let me know if any further changes are needed. Thank you

[petrasovaa]

This is a lot of code changes, needs more substantial review.

[echoix]

I’m still on vacation, didn’t have time to read through, but I will probably need to examine on a computer. It seems this final state has a lot of different little changes. If there’s anything that isn’t strictly needed to make it work, you could remove to keep the diff the smallest possible while still doing what’s needed, it’ll help review. Small, concise PRs where there’s only one risk is way easier to understand review and merge.

[SyedAhad01]

Hi @echoix and @petrasovaa, thank you for the feedback and for taking the time to look at this despite being on vacation.

I understand the PR has grown quite large. I'll go through it carefully and split it into smaller, more focused PRs to make review easier. I'll also remove anything that isn't strictly necessary to keep the diff minimal.

Although I wasn't selected for GSoC, I genuinely enjoy contributing to GRASS GIS and would love to keep working on this and future issues especially around testing/CI. If you have any suggestions on where I can contribute next, I'd be happy to take them up.

Thanks again for your patience and support.

[wenzeslaus]

This changes and touches too many things, while declaring and allowing for much smaller scope.

@SyedAhad01 If you want to revisit this, now would be the time, but if proposes as is, I suggest closing without a merge.

AI-generated review

State as of 35140be (merge of main on 2026-09-29): 4 files changed, +174/-61 against main. Since the 2026-04-30 plan to split the PR and keep only what is strictly needed (following the 2026-04-29 comments from @petrasovaa and @echoix), the branch has received only merges from main and one applied one-line suggestion.

Relation to main. #7724 (merged 2026-09-24) added grass.script.utils._get_multiprocessing_context(). It pins the "fork" start method where available (not on macOS) because GRASS worker processes rely on fork semantics, and Python 3.14 switched the Linux default to "forkserver". This PR goes the opposite way and forces "spawn" on all platforms, both in GridModule.run and in the tests. The tests' old xfail_mp_spawn condition (get_start_method() == "spawn") does not cover "forkserver", which explains the 3.14 failures on Linux. An option is a separate minimal PR that uses _get_multiprocessing_context() in GridModule.run and in the tests' run_in_subprocess, consistent with #7724.

grid.py

1. GridModule.run now always uses mltp.get_context("spawn"), replacing the platform default. On Linux, a user script that calls GridModule(...).run() at module level without an if __name__ == "__main__": guard now breaks: a minimal spawn Pool script without the guard raises the "bootstrapping phase" RuntimeError and hangs (checked locally with Python 3.12). Every worker also starts a new interpreter and re-imports grass. The branch's own commit 834728b "remove forced spawn context, keep pickling fixes" is not reflected in the final state.
2. ctx.set_executable(sys.executable) is process-global: BaseContext.set_executable calls multiprocessing.spawn.set_executable. It overrides an executable set by an embedding application (for example, one where sys.executable is not a Python interpreter), and the change affects all later spawn-based multiprocessing in that process.
3. The new result.get() reporting only surfaces exceptions raised in cmd_exe. cmd_exe still ignores the return codes of g.region and of the tool (Popen(...).wait()), so a tool failing in a tile still does not raise. This is pre-existing on main, but the branch's commit 6cb8b92 "check return code in cmd_exe" is not in the final diff. The message also changed from a translatable _("...") string to an f-string.
4. _grass_worker_init checks gisbase not in sys.path (not etc_python), then repeats the check for etc_python. A spawned child already receives the parent's sys.path, so in normal use the initializer has no effect. When GISBASE is not set, it does nothing.
5. The list() wrappers do not change behavior: Module.get_dict() already returns lists for inputs and outputs, Bbox.items() returns a list, and the list(inputs.items()) in cmd_exe is created inside the worker and never pickled. The new_mset 1-tuple was harmless too: write_gisrc writes "MAPSET: %s\n" % mapset, and "%s" % ("tile",) gives tile. These are cleanups, not pickling fixes.

module.py

6. Nothing uses the new Module.__getstate__/__setstate__: GridModule sends get_dict() output to the workers, and the tests pass only plain values through functools.partial, so no Module instance is pickled. __setstate__ repeats the default behavior.

Tests

7. test_cleans with non_exist_surface disables the exit code check. This was already raised in #7257 (comment) and is still open: any failure in the child (import, pickling) passes for those parametrizations, because the test only asserts on mapset directories.
8. In _run_grid_module, the loop inserting grass_python_paths into sys.path never inserts anything, because the paths come from sys.path itself. It then sets PYTHONPATH to the whole sys.path, which is passed to every GRASS tool the child starts. That reintroduces the source-checkout paths _run_with_env removes from sys.path. The function also re-imports sys, os and grass.script locally, although the module already imports sys and grass.script.
9. _run_with_env (rewriting sys.path, purging grass.* from sys.modules, adding DLL directories) is about 50 lines that exist only to make the forced "spawn" work from a source checkout. With the fork context from #7724, the child inherits the parent's state as on main, and none of it is needed.
10. The child calls gs.setup.init(project) without a context manager or finish(), so the session it creates is never cleaned up.

CI. The pytest.yml change (3.13 to 3.14) was requested in review and is fine on its own. It could go in the minimal PR so the 3.14 fix is tested.