lukassnoek / pybest

PYthon package for Beta ESTimation (of single-trial fMRI data)
MIT License
7 stars 4 forks source link

Change in fmriprep confounds filename #9

Open mszinte opened 3 years ago

mszinte commented 3 years ago

While running it back with nifti files, i found an error due to a change with new vesion of fmriprep. Indeed nowadays filenames of fmriprep confounds files are ending with : _timeseries.tsv I changed the bookkeeping.py function to agree with this new naming.

lukassnoek commented 3 years ago

@tknapen , do you want pybest to be "backwards compatible" with old versions of Fmriprep? If so, perhaps we could include a try/except block which checks for either confounds idenfitier.

tknapen commented 3 years ago

Yeah, this is a tricky point. If we keep changing pybest code to track fmriprep's changing output filenames that will be awful for backwards compatibility and general stability -- you'll just have to keep changing the code with every new fmriprep version, and this will immediately break pybest's functionality on older datasets.

I envisage the following as a long-term solution. It would be optimal if we could create a .yml file that lists, for every fmriprep version, the regular expressions to be used to capture the different output types. Then this fmriprep version used to preprocess the data can either be a command line argument, or read from the fmriprep output directory directly. It requires some coding but should be a stable solution. I'm not going to have the time to implement this right now, though.

julfou81 commented 3 years ago

Hi, regarding this point, here is what was done in the XCPEngine (https://github.com/PennLINC/xcpEngine) code:

https://github.com/a3sha2/xcpEngine/commit/fc288a4e9a08fe861f331fc45a4f903c0d8c2e5e

gjheij commented 2 years ago

Aside from the confounds_regressors or confounds_timeseries (which was introduced in version >20), fMRIPrep also switched around the hemi-* and space-* flags in version >21. So for fMRIPrep v20.2.0, the output looked like sub-005_ses-1_task-2R_run-2_space-fsnative_hemi-L_bold.func.gii, while with v21 it looks like sub-01_ses-1_task-AS0_run-1_hemi-L_space-fsnative_bold.func.gii.

I now have this in in find_exp_parameters:

    with open(op.join(cfg['fprep_dir'], 'dataset_description.json')) as conf:
        info = json.load(conf)
    cfg['version'] = info['GeneratedBy'][0]['Version']

    if int(cfg['version'].split('.')[0]) < 21:
        cfg['space_idf'] = f'hemi-{hemi}*.func.gii'
        cfg['hemi_idf'] = f'space-{space}'
    else:
        cfg['space_idf'] = f'space-{space}*.func.gii'
        cfg['hemi_idf'] = f'hemi-{hemi}'

and this in find_data:

    if int(cfg['version'].split('.')[0]) < 20:
        globber = "confounds_regressors"
    else:
        globber = "confounds_timeseries"

The yml-file would be nice, but fMRIPrep doesn't change file names that often. So for now it seems sufficient to have a block that tests for version > 21.

This dataset_description solution is still rather hairy, as it gets updated every time you instantiate fMRIPrep. Thus, if you mix versions, you're in trouble.

gjheij commented 2 years ago

Another option is to remain agnostic about the order of hemi-* and space-* flags by using this function. You can just give it a list of substrings you want to filter for without a specific order.

lukassnoek commented 2 years ago

Ugh, so much for fMRIPREP's "stable API"... I'd recommend sticking to the most recent version of fMRIPREP and force users of this package to just upgrade their fMRIPREP version :)