nipreps/niworkflows

Non-compliant CIFTI sidecar fields clashing with entities

effigies opened this issue · 4 comments

effigies commented

Since #750 this section is generating metadata that is now being dumped into the space-fsLR_bold.json files:

niworkflows/niworkflows/interfaces/cifti.py

Lines 261 to 324 in 959bed7

def _get_cifti_variant(surface, volume, density=None):
"""
Identify CIFTI variant and return metadata.
Parameters
----------
surface : str
Target surface space
volume : str
Target volume space
density : str, optional
Surface density (required for `fsLR` surfaces)
Returns
-------
out_metadata : str
JSON file with variant metadata
variant : str
Name of CIFTI variant
Examples
--------
>>> metafile, variant, _ = _get_cifti_variant('fsaverage5', 'MNI152NLin2009cAsym')
>>> str(metafile) # doctest: +ELLIPSIS
'.../dtseries_variant.json'
>>> variant
'fMRIPrep grayordinates'
>>> _, variant, grayords = _get_cifti_variant('fsLR', 'MNI152NLin6Asym', density='59k')
>>> variant
'HCP grayordinates'
>>> grayords
'170k'
"""
if surface in ("fsaverage5", "fsaverage6"):
density = {"fsaverage5": "10k", "fsaverage6": "41k"}[surface]
surface = "fsaverage"
for variant, targets in CIFTI_VARIANTS.items():
if all(target in targets for target in (surface, volume)):
break
variant = None
if variant is None:
raise NotImplementedError(
"No corresponding variant for (surface: {0}, volume: {1})".format(
surface, volume
)
)
grayords = None
out_metadata = Path.cwd() / "dtseries_variant.json"
out_json = {
"space": variant,
"surface": surface,
"volume": volume,
"surface_density": density,
}
if surface == "fsLR":
grayords = {"32k": "91k", "59k": "170k"}[density]
out_json["grayordinates"] = grayords
out_metadata.write_text(json.dumps(out_json, indent=2))
return out_metadata, variant, grayords

These keys have no basis in BIDS, and the space key clashes with the space entity. I'm not sure if we should just drop it. Or we could bundle the whole group into a custom SpaceDescription object.

Example failure here:

Traceback (most recent call last):
  File "/opt/conda/bin/fmriprep", line 8, in <module>
    sys.exit(main())
  File "/opt/conda/lib/python3.9/site-packages/fmriprep/cli/run.py", line 185, in main
    failed_reports = generate_reports(
  File "/opt/conda/lib/python3.9/site-packages/fmriprep/reports/core.py", line 96, in generate_reports
    report_errors = [
  File "/opt/conda/lib/python3.9/site-packages/fmriprep/reports/core.py", line 97, in <listcomp>
    run_reports(
  File "/opt/conda/lib/python3.9/site-packages/fmriprep/reports/core.py", line 79, in run_reports
    return Report(
  File "/opt/conda/lib/python3.9/site-packages/niworkflows/reports/core.py", line 291, in __init__
    self._load_config(Path(config or pkgrf("niworkflows", "reports/default.yml")))
  File "/opt/conda/lib/python3.9/site-packages/fmriprep/reports/core.py", line 48, in _load_config
    self.index(settings["sections"])
  File "/opt/conda/lib/python3.9/site-packages/niworkflows/reports/core.py", line 322, in index
    self.init_layout()
  File "/opt/conda/lib/python3.9/site-packages/niworkflows/reports/core.py", line 313, in init_layout
    self.layout = BIDSLayout(self.root, config="figures", validate=False)
  File "/opt/conda/lib/python3.9/site-packages/bids/layout/layout.py", line 152, in __init__
    indexer(self)
  File "/opt/conda/lib/python3.9/site-packages/bids/layout/index.py", line 111, in __call__
    self._index_metadata()
  File "/opt/conda/lib/python3.9/site-packages/bids/layout/index.py", line 438, in _index_metadata
    raise BIDSConflictingValuesError(
bids.exceptions.BIDSConflictingValuesError: Conflicting values found for entity 'space' in filename /out/fmriprep/sub-01/func/sub-01_task-mixedgamblestask_run-02_space-fsLR_den-91k_bold.dtseries.nii (value='fsLR') versus its JSON sidecar (value='HCP grayordinates'). Please reconcile this discrepancy.
fMRIPrep: Please report errors to https://github.com/nipreps/fmriprep/issues

Exited with code exit status 1
effigies commented

Are we okay with dropping this metadata for now, and then making an issue to populate SpatialReference?

mgxd commented

Isn't it easy enough to drop in favor of SpatialReference in the same PR?

effigies commented

Closed by #756.