PennLINC / qsirecon

Reconstruction of preprocessed q-space images (dMRI)
https://qsirecon.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
5 stars 3 forks source link

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

Open mattcieslak opened 4 days ago

mattcieslak commented 4 days ago

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
tsalo commented 4 days ago

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

https://github.com/PennLINC/qsirecon/blob/0b53f5338e635e307d3b21d1f91b9e6ea7328eb6/qsirecon/cli/parser.py#L320-L326

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)