Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The error message for not specifying --fs-license-file is unintuitive #183

Open
mattcieslak opened this issue Nov 21, 2024 · 1 comment
Open
Labels
enhancement New feature or request
Milestone

Comments

@mattcieslak
Copy link
Contributor

I forgot to add --fs-license-file to a recent run of QSIRecon and ran into:

  File "/opt/conda/envs/qsiprep/lib/python3.10/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/opt/conda/envs/qsiprep/lib/python3.10/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "/opt/conda/envs/qsiprep/lib/python3.10/site-packages/qsirecon/cli/workflow.py", line 121, in build_workflow
    if not Path(config.execution.fs_license_file).exists():
  File "/opt/conda/envs/qsiprep/lib/python3.10/pathlib.py", line 960, in __new__
    self = cls._from_parts(args)
  File "/opt/conda/envs/qsiprep/lib/python3.10/pathlib.py", line 594, in _from_parts
    drv, root, parts = self._parse_args(args)
  File "/opt/conda/envs/qsiprep/lib/python3.10/pathlib.py", line 578, in _parse_args
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

@mattcieslak mattcieslak added the enhancement New feature or request label Nov 21, 2024
@tsalo
Copy link
Member

tsalo commented Nov 21, 2024

We currently have the type set to Path. We should probably set it to IsFile (i.e., an existing file):

g_fs.add_argument(
"--fs-license-file",
metavar="PATH",
type=Path,
help="Path to FreeSurfer license key file. Get it (for free) by registering "
"at https://surfer.nmr.mgh.harvard.edu/registration.html",
)

When a user doesn't provide it at all, we can do what XCP-D used to do (see here):

    if opts.fs_license_file is not None:
        opts.fs_license_file = opts.fs_license_file.resolve()
        if opts.fs_license_file.is_file():
            os.environ["FS_LICENSE"] = str(opts.fs_license_file)

        else:
            error_messages.append(f"Freesurfer license DNE: {opts.fs_license_file}.")
    else:
        fs_license_file = os.environ.get("FS_LICENSE", "/opt/freesurfer/license.txt")
        if not Path(fs_license_file).is_file():
            error_messages.append(
                "A valid FreeSurfer license file is required. "
                "Set the FS_LICENSE environment variable or use the '--fs-license-file' flag."
            )

        os.environ["FS_LICENSE"] = str(fs_license_file)

@tsalo tsalo added this to the 1.1.0 milestone Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants