ENH: Add options and outputs to fsl.Eddy interface

[josephmje]

Summary

Updates interface for FSL's eddy command with inputs and outputs for FSL 5.0.10+

Splits #3008 into 2 PRs.

List of changes proposed in this PR (pull-request)

• add defaults for some input parameters
• add additional inputs and outputs pertaining to outlier replacement, intra-volume movement correction and susceptibility-by-movement correction (outlier maps necessary for eddy_quad)
• requires fieldmap to be trait File instead of Str (would break backwards compatibility)

Acknowledgment

• (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

[josephmje]

I just realized that slice-to-volume motion correction is only implemented when use_cuda is True. The options that are included in this are: mporder, s2v_niter, s2v_lambda, s2v_interp, slspec and json. Any ideas on how to add a check for this?

[effigies]

requires=['use_cuda']?

[josephmje]

use_cuda can be made to be True or False. Does requires=['use_cuda']work if use_cuda is False?

[effigies]

I'd check!

[josephmje]

Cool! Thanks, I'll try it out

[codecov]

Codecov Report

Merging #3034 into master will increase coverage by 0.61%.
The diff coverage is 56%.

@@            Coverage Diff             @@
##           master    #3034      +/-   ##
==========================================
+ Coverage   67.23%   67.84%   +0.61%     
==========================================
  Files         294      295       +1     
  Lines       39245    39310      +65     
  Branches     5162     5181      +19     
==========================================
+ Hits        26386    26671     +285     
+ Misses      12185    11933     -252     
- Partials      674      706      +32

Flag	Coverage Δ
#smoketests	51.23% <56%> (ø)	⬆️
#unittests	65.07% <56%> (+0.85%)	⬆️

Impacted Files	Coverage Δ
nipype/interfaces/fsl/epi.py	62.15% <56%> (-0.92%)	⬇️
nipype/pipeline/plugins/legacymultiproc.py	62.81% <0%> (-0.51%)	⬇️
nipype/testing/__init__.py	88.88% <0%> (ø)
nipype/interfaces/spm/preprocess.py	56.93% <0%> (+0.32%)	⬆️
nipype/utils/filemanip.py	77.59% <0%> (+0.47%)	⬆️
nipype/interfaces/freesurfer/preprocess.py	66.04% <0%> (+0.98%)	⬆️
nipype/interfaces/freesurfer/model.py	64.19% <0%> (+1.12%)	⬆️
nipype/interfaces/spm/base.py	87% <0%> (+2%)	⬆️
nipype/interfaces/freesurfer/utils.py	62.88% <0%> (+2.13%)	⬆️
nipype/interfaces/io.py	59.67% <0%> (+6.11%)	⬆️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0225a45...42d00c7. Read the comment docs.

[josephmje]

requires=['use_cuda'] doesn't quite work. The doctests will still pass if I make use_cuda = False.

Actually, all of the requires might need some finessing.
requires=['repol'] actually needs repol = True
requires=['mporder'] needs mporder > 0
requires=['estimate_move_by_susceptibility'] needs estimate_move_by_susceptibility = True

[effigies]

Most of these comments are checking whether the defaults that are now being set are actually defaults, and most of the rest are on clarity of desc strings or trait names.

I'm a little surprised that requires for a traits.Bool doesn't require the value to be True. You could change them to traits.Enum(True), which are either True or Undefined. It's not very pleasing though...

[josephmje]

converting some of the former Bool traits to Enum ended up passing the doctests but fail to run in a workflow:

  File "/scratch/mjoseph/.pyenv/versions/dmriprep_venv/lib/python3.7/site-packages/nipype-1.2.3-py3.7.egg/nipype/interfaces/base/core.py", line 801, in _format_arg
    return argstr % value
TypeError: not all arguments converted during string formatting

[effigies]

Hmm. This is somewhat frustrating. I guess I'd revert to traits.Bool, and then just raise a warning (or an error, if you think it's worth it) in _format_arg() for those that require a Bool that's False, and then return None.

[effigies]

Hi @josephmje, sorry to do this to you, but we just merged some pretty huge changes. Could you re-apply these changes on the latest master? You'll want to re-run make specs entirely, and we're now using the Black formatter. You can run it manually, or use pre-commit to have it automatically run every time you commit.

Please let me know if you need any help or clarifications.

[josephmje]

@effigies Sorry my rebase didn't go so well. Do you know what I can do to fix this?

[effigies]

I'll see what I can do.

[effigies]

I rebased, selecting your changes, and running black/make specs before committing each revision. It looks like this doubled up a bunch of commits. I'm not really clear on how that happened.

To update your local branch use git fetch origin && git reset --hard origin/update_eddy.

[effigies]

Thanks for this.