[FIX] - fix bidsify match algorithm associating the wrong fieldmaps

[jerdra]

This is a bugfix for the bidsify BIDS conversion script.

The issue

The way that bidsify works is by reading in assumptions for how fmaps should be matched within the datman config file using the pair key within the config yaml. However bidsify provides erroneous outputs when:

I.e, the OPT dwi dir-AP TOPUP requires a pair (dir-PA), however for UT1 it does not - the b0 acts as the pair. However the OPTIMUM scanning sequence is such that the next fmap is a dir-PA meant for the faces task. As a result, bidsify erroneously pairs the dwi-AP topup with the fmri-PA topup.

The Fix

The fix is to include the intendedfor key in the matching protocol so we don't accidentally match dwi topup to func topup

In addition, the script has been updated to rely more on site-specific configuration if needed. Meaning if we expect a lone-DWI topup it should be explicitly specified in config. If not, bidsify will raise a logging error that can be over-ridden with --allow-incomplete.

Data consequences

1. OPTIMUM FACES task data should be re-processed (it should've failed for cases in which there is a single diffusion TOPUP)
2. Diffusion may have to be re-run if the BIDS data in the archive was used

Comments needed for: Breaking 3.6 compatibility

One issue with this PR is that I used the Python3.7 dataclasses module to clean up the fmap matching algorithm. This breaks 3.6 compatibility, are we okay with this? If not, I can just refactor the FMapMatches dataclass to a regular class.

[pep8speaks]

Hello @jerdra, Thank you for updating!

• In the file bin/bidsify.py:

Line 326:17: E128 continuation line under-indented for visual indent

To test for issues locally, pip install flake8 and then run flake8 datman.

Comment last updated at 2021-02-03 18:01:19 UTC

[DESm1th]

I have no problem with breaking the backward compatibility with 3.6. Aside from having to update our circleci build (which runs against 3.6) I can't really forsee any issues with doing so.

[benselby]

Looks good to my eyes - nice work!

[jerdra]

thankss for the input friends, @DESm1th i'll update circleCI to reflect that.

Just working through all of OPTIMUM atm to make sure no unexpected cases fall through the cracks. If OPTIMUM checks out I think we're safe to merge

[jerdra]

@benselby @DESm1th @josephmje conversion checks out, no more silently converting problematic fmaps! I'll be pushing updates to datman-config as well since bidsify won't convert if lone fmaps are not expected (dMRI scans w/only 1 topup)

[jerdra]

Thanks Dawn, the build_docs failed due to authentication errors (probs because its my PR branch). I think we should be okay to go! Can someone @benselby @josephmje review/approve whenever you get the chance?