ENH: improve FreeSurfer error message when executable not found

[ayuclan]

Closes #12917

Improved error message when FreeSurfer executable (e.g., mri_watershed) cannot be found.

• Clarifies need for proper FreeSurfer setup
• Mentions adding $FREESURFER_HOME/bin to PATH
• Notes that Python/Jupyter should be started from configured shell

This helps users diagnose common setup issues more easily.

[ayuclan]

Hi, this is my first contribution to MNE.

I’ve added a clearer error message for missing FreeSurfer executables. I also saw pre-commit auto-fixes applied—please let me know if any further changes or formatting adjustments are needed.

Thanks!

[wmvanvliet]

If we don't opt for some magic solution where MNE-Python tries to go the FreeSurfer setup itself, then this is indeed a much better error message. Thanks @ayuclan!

[JiwaniZakir]

The error handling in bem.py catches FileNotFoundError specifically for the mri_watershed executable, but the error message hardcodes that name rather than referencing the actual command from cmd[0]. If run_subprocess_env raises FileNotFoundError for a different reason — e.g., an input file path issue on some platforms — the message will be misleading. Consider either extracting the executable name dynamically (cmd[0]) or at minimum catching the exception as e and checking e.filename to verify it matches the expected executable before reraising with the custom message. Additionally, it's worth adding a test case that mocks run_subprocess_env to raise FileNotFoundError and asserts that the resulting RuntimeError message contains the expected guidance text, since this new branch is currently untested. The documentation link at the end of the message ("See MNE installation documentation for details.") would be more useful if it included an actual URL.

[ayuclan]

The error handling in bem.py catches FileNotFoundError specifically for the mri_watershed executable, but the error message hardcodes that name rather than referencing the actual command from cmd[0]. If run_subprocess_env raises FileNotFoundError for a different reason — e.g., an input file path issue on some platforms — the message will be misleading. Consider either extracting the executable name dynamically (cmd[0]) or at minimum catching the exception as e and checking e.filename to verify it matches the expected executable before reraising with the custom message. Additionally, it's worth adding a test case that mocks run_subprocess_env to raise FileNotFoundError and asserts that the resulting RuntimeError message contains the expected guidance text, since this new branch is currently untested. The documentation link at the end of the message ("See MNE installation documentation for details.") would be more useful if it included an actual URL.

Thanks for the detailed feedback. I’ll work on these changes and update the PR soon.

[wmvanvliet]

I think @ayuclan's approach is good. For one, all the input parameters are already explicitly checked by the code. Second, if something fails in the mri_watershed command, such as an input file path being wrong, we raise a CalledProcessError, not a FileNotFoundError. To be extra safe, we could use the raise ... from construction to also have the original FileNotFoundError in the error message. In the unlikely event the error was not raised because mri_watershed is not in the path, it will show up. I'll add a comment with the suggested change.

@JiwaniZakir I know you use AI and not always disclose it. Was your comment AI generated?

[wmvanvliet]

I would be ok with not adding a unit test for this one, because it would be a little tricky. The unittest we have for make_watershed_bem has a @requires_freesurfer("mri_watershed") decorator which makes sure the test only runs if freesurfer has been installed correctly. So we can't test for installation issues inside that test :) I'm not a fan of mocking run_subprocess_env to return a FileNotFoundError, because what we would want to test is that we get the proper error message when we have an actual installation issue.

[ayuclan]

Thank you for the review and approval! I’ve addressed the suggestions and added the changelog entry. Please let me know if anything else is needed.

[wmvanvliet]

The style checker is complaining:

 
E501 Line too long (98 > 88)
    --> mne/bem.py:1325:89
     |
1323 |             "- FREESURFER_HOME is set\n"
1324 |             "- $FREESURFER_HOME/bin is in your PATH\n"
1325 |             "- You started Python/Jupyter from a terminal where SetupFreeSurfer.sh is sourced\n\n"
     |                                                                                         ^^^^^^^^^^
1326 |             "See MNE installation documentation for details."
1327 |         ) from e
     |

looks like line 1325 is too long now and needs to be broken up.

Also, if you create an account on circleci, then the documentation building bot can run for you and see if your changelog entry renders properly.

[wmvanvliet]

Also, to give you proper credit, can you add your name to doc/names.inc, and end your changelog entry with , by `Ayushi Satodiya`_ ? (see any of the other changelog entries on how we always end them with our names).

[ayuclan]

Thanks for pointing that out! I’ve fixed the line length issue in bem.py and updated the changelog with my name. Please let me know if anything else needs adjustment.

…

On Sun, Apr 12, 2026 at 4:46 PM Marijn van Vliet ***@***.***> wrote: *wmvanvliet* left a comment (mne-tools/mne-python#13790) <#13790 (comment)> Also, to give you proper credit, can you add your name to doc/names.inc, and end your changelog entry with , by `Ayushi Satodiya`_ ? (see any of the other changelog entries on how we always end them with our names). — Reply to this email directly, view it on GitHub <#13790 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJHAVSVUH3N7RNE5VMES5L34VN3KRAVCNFSM6AAAAACXAKPAN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DEMZRGM4DKMJRGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[wmvanvliet]

Let's see if we can make all the checkmarks green :) Currently, circleci refuses to run because you have not registered an account with them yet. Could you click on one of the failed ci/circleci tests and then signup for circleci?

[ayuclan]

Hello, I’ve set up CircleCI and re-triggered the checks. The CircleCI jobs (build_docs and linkcheck) are now running successfully. There is currently one remaining test failure on Python 3.14 ( test_3d_backend[pyvistaqt]), related to a VTK API change (GetCapacity vs Capacity). I’m working on updating this to ensure compatibility. I’ll push a fix shortly once verified.

…

On Mon, Apr 13, 2026 at 4:43 PM Marijn van Vliet ***@***.***> wrote: *wmvanvliet* left a comment (mne-tools/mne-python#13790) <#13790 (comment)> Let's see if we can make all the checkmarks green :) Currently, circleci refuses to run because you have not registered an account with them yet. Could you click on one of the failed ci/circleci tests and then signup for circleci? — Reply to this email directly, view it on GitHub <#13790 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJHAVSR2PSGZWHR7GBN4UYL4VTDWPAVCNFSM6AAAAACXAKPAN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DEMZVHE2DCMRWG4> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ayuclan]

Hello,

I’ve set up CircleCI and re-triggered the checks—those are now passing.

There’s one remaining failure on Python 3.14 (test_3d_backend[pyvistaqt]), likely due to a VTK API change (GetCapacity vs Capacity). I couldn’t find any direct usage in the repo, so it seems dependency-related.

Should I handle this, or ignore for now?

[drammock]

There’s one remaining failure on Python 3.14 (test_3d_backend[pyvistaqt]), likely due to a VTK API change (GetCapacity vs Capacity). I couldn’t find any direct usage in the repo, so it seems dependency-related.

Should I handle this, or ignore for now?

preferable that this gets fixed in a separate PR, since it's not related to the changes here. If you know how to fix it, feel free to open a new PR for it --- if not, someone else will do so soon.

[ayuclan]

Got it, thank you ! I’ll leave this for now since it’s not related to this PR. I’ll try to look into it separately.

…

Message ID: ***@***.***>

[JiwaniZakir]

The exception chaining suggestions from wmvanvliet (except FileNotFoundError as e + raise ... from e) are important to keep — they preserve the original traceback so users can see exactly which executable triggered the error. Make sure both suggestions are applied together, otherwise from e will reference an unbound variable.

[ayuclan]

Hi @wmvanvliet and @drammock , I rebased the branch to fix the earlier merge issue and force-pushed the updated history.

It looks like the workflows are now waiting for approval and some checks are still showing previous failures.

Could you please approve/re-run the workflows when you have time?

[ayuclan]

The exception chaining suggestions from wmvanvliet (except FileNotFoundError as e + raise ... from e) are important to keep — they preserve the original traceback so users can see exactly which executable triggered the error. Make sure both suggestions are applied together, otherwise from e will reference an unbound variable.

Thanks for pointing this out but Both changes are already applied — the exception is caught as “FileNotFoundError as e” and re-raised using “from e” to preserve the original traceback.

[JiwaniZakir]

The improved error message is clear and actionable — pointing users toward $FREESURFER_HOME/bin in PATH and the shell-launch requirement covers the two most common setup pitfalls. One minor consideration: if FREESURFER_HOME isn't set at all, the message referencing it might still confuse users who haven't sourced the FreeSurfer setup script yet — worth verifying the message handles that case gracefully. Otherwise the change looks good to merge.

[JiwaniZakir]

Good to know SetupFreeSurfer.sh is still the correct name — I've kept it as-is then. I've applied your suggestions for catching FileNotFoundError as e and chaining the exception with from e, which gives a cleaner traceback for users debugging setup issues.

[wmvanvliet]

@ayuclan could you add yourself to doc/changes/names.inc?

[ayuclan]

I’ve added my name to doc/changes/names.inc and double-checked it is present now. Please let me know if I missed anything.

…

On Fri, Apr 17, 2026 at 5:22 PM Marijn van Vliet ***@***.***> wrote: *wmvanvliet* left a comment (mne-tools/mne-python#13790) <#13790 (comment)> @ayuclan <https://github.com/ayuclan> could you add yourself to doc/changes/names.inc? — Reply to this email directly, view it on GitHub <#13790 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJHAVSUVE2A5QCWXQWLEW334WILIPAVCNFSM6AAAAACXAKPAN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DENRXG4ZTMNZSGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[JiwaniZakir]

The test failures due to the warning itself rather than computation differences are a real concern — if tests are asserting on stderr/stdout or using warning filters, adding a new warning will break them regardless of correctness. It might be worth auditing which specific tests failed to determine if they need updating or if the warning text/category should be adjusted to avoid triggering existing pytest.warns or -W error filters in the test suite.

[ayuclan]

I’ve added my name to doc/changes/names.inc and confirmed it’s included in the PR. Please let me know if anything else is needed.

…

On Fri, Apr 17, 2026 at 5:22 PM Marijn van Vliet ***@***.***> wrote: *wmvanvliet* left a comment (mne-tools/mne-python#13790) <#13790 (comment)> @ayuclan <https://github.com/ayuclan> could you add yourself to doc/changes/names.inc? — Reply to this email directly, view it on GitHub <#13790 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJHAVSUVE2A5QCWXQWLEW334WILIPAVCNFSM6AAAAACXAKPAN2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DENRXG4ZTMNZSGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[ayuclan]

The test failures due to the warning itself rather than computation differences are a real concern — if tests are asserting on stderr/stdout or using warning filters, adding a new warning will break them regardless of correctness. It might be worth auditing which specific tests failed to determine if they need updating or if the warning text/category should be adjusted to avoid triggering existing pytest.warns or -W error filters in the test suite.

From the CI logs, the current failures appear related to the pyvistaqt / VTK issue on Python 3.14 rather than changes introduced in this PR.

This PR only modifies the error handling when the FreeSurfer executable is not found and does not introduce new warnings in normal execution paths.

Please let me know if you’d like me to investigate any specific failing tests further.

[JiwaniZakir]

The improvement looks useful — one thing worth considering is whether the error message should also check if $FREESURFER_HOME is set at all, since that's often the root cause when the executable isn't found, and distinguishing "env var not set" from "bin not in PATH" would make diagnosis even faster. Also, don't forget to add yourself to doc/changes/names.inc as @wmvanvliet mentioned.