lib/python/pygrass: fix Python 3.14 pickling error in grid tests

[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

[echoix]

Suggested change

[echoix]

Suggested change

[echoix]

Suggested change

[echoix]

Suggested change

[echoix]

Suggested change

[echoix]

Suggested change

[echoix]

That logic here is unexpected to me, could you explain a bit?

[SyedAhad01]

I added this condition to handle the case where the surface does not exist. On Windows/macOS (spawn), the subprocess always fails, so we let the error propagate. On Linux (fork), we skip the error so the test can still check if mapsets were cleaned. Let me know sir if there is a simpler way to do this

[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