PennLINC / qsiprep

Preprocessing of diffusion MRI
http://qsiprep.readthedocs.io
BSD 3-Clause "New" or "Revised" License
138 stars 55 forks source link

The inheritance principle is not completely supported #685

Open tsalo opened 7 months ago

tsalo commented 7 months ago

I haven't done anything near a complete inventory of where metadata is loaded in QSIPrep, but in at least one location (get_metadata_for_nifti), inherited metadata is not support.

I think the best solution is to use a BIDSLayout to load metadata instead of directly finding the (potentially multiple) JSONs associated with a NIFTI. This might be easier if we implement the Nipreps Config object.

tsalo commented 7 months ago

Found another case, where bval and bvec files are grabbed based on the DWI filename. I'm not sure where in the code this happens (other than that it's in the merge workflow), but that seems to be the case.

Per chat with @mattcieslak, bvals should vary by run, but inheritance should definitely apply to magnitude and phase images, which come from the same run. So you'd typically have sub-X_dwi.bval for sub-X_part-mag_dwi.nii.gz and sub-X_part-phase_dwi.nii.gz.

akimbler commented 6 months ago

Partially related to this, I've noticed that when running qsirecon. I frequently get outputs that seem to be jammed together combinations of other input filenames with updated entities appended, rather than proper inheritance via entities. image image

mattcieslak commented 6 months ago

I agree, the qsirecon output names need a major overhaul. That's underway in #688 and will be in the next release. We're going to keep it close to BEP016

tsalo commented 3 months ago

760 removes some of the JSON-loading functions, but doesn't deal with read_nifti_sidecar, which is a big barrier to using the inheritance principle in DWI datasets.