Skip to content

ENH: improve FreeSurfer error message when executable not found#13790

Open
ayuclan wants to merge 4 commits intomne-tools:mainfrom
ayuclan:fix-freesurfer-error-message
Open

ENH: improve FreeSurfer error message when executable not found#13790
ayuclan wants to merge 4 commits intomne-tools:mainfrom
ayuclan:fix-freesurfer-error-message

Conversation

@ayuclan
Copy link
Copy Markdown

@ayuclan ayuclan commented Mar 26, 2026

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 and others added 3 commits March 26, 2026 17:31
Closes mne-tools#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 ayuclan closed this Mar 26, 2026
@ayuclan ayuclan reopened this Mar 26, 2026
@ayuclan
Copy link
Copy Markdown
Author

ayuclan commented Mar 26, 2026

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
Copy link
Copy Markdown
Contributor

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!

ENH: refine FreeSurfer setup error message

Co-authored-by: Marijn van Vliet <w.m.vanvliet@gmail.com>
Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

Choose a reason for hiding this comment

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

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
Copy link
Copy Markdown
Author

ayuclan commented Mar 30, 2026

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
Copy link
Copy Markdown
Contributor

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?

"- $FREESURFER_HOME/bin is in your PATH\n"
"- You started Python/Jupyter from a terminal where SetupFreeSurfer.sh is sourced\n\n"
"See MNE installation documentation for details."
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
)
) from e

run_subprocess_env(cmd)
try:
run_subprocess_env(cmd)
except FileNotFoundError:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
except FileNotFoundError:
except FileNotFoundError as e:

@wmvanvliet
Copy link
Copy Markdown
Contributor

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide more helpful error message if FreeSurfer executables cannot be found; consider amending install docs

3 participants