[FIX] modified afni's cat_matvec to accept empty string opposed to opkey

[ostanley]

Summary

AFNI's cat_matvec allows for transforms to be concatenated together. The opkey field after each transform allows for things like inversions but is non-mandatory. Currently, if no opkey is specified the - character is added resulting in an error when the interface is run.
Example:
cat_matvec a.1D - b.1D - > c.1D

Reference documentation:
https://afni.nimh.nih.gov/pub/dist/doc/program_help/cat_matvec.html

Fixes

• The interface is not able to perform if no opkey is specified aka a list of tuples which are ('file.1D',''). I have fixed this at the format args level.
• New behaviour: if an empty opkey is found then the - character is not added.

Acknowledgment

I acknowledge that this contribution will be available under the Apache 2 license.

[effigies]

Thanks for this, I made some suggestions.

[effigies]

I think it would be clearer to continue writing this as a comprehension, using the x if y else z idiom, but we can use more verbose variable names to make the logic clearer. I also think it would be useful to include a comment:

# Concatenate a series of filenames, with optional opkeys
return ' '.join('%s -%s' % (mfile, opkey) if opkey else mfile
                for mfile, opkey in value)

I dropped the initial spec.argstr %, just because spec.argstr is "%s", so there's no real point, but you could keep it if you wanted.

[ostanley]

I agree this is more elegant, I changed it and confirmed it passed my test for formatting.

[effigies]

Can you re-add this? Python style guidelines suggest 2 lines between classes.

[ostanley]

Done

[codecov]

Codecov Report

Merging #2943 into master will increase coverage by 0.37%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2943      +/-   ##
==========================================
+ Coverage   67.14%   67.52%   +0.37%     
==========================================
  Files         344      344              
  Lines       44091    44091              
  Branches     5556     5556              
==========================================
+ Hits        29606    29771     +165     
+ Misses      13703    13574     -129     
+ Partials      782      746      -36

Flag	Coverage Δ
#smoketests	50.3% <ø> (+1.36%)	⬆️
#unittests	64.97% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
nipype/interfaces/afni/utils.py	81.89% <100%> (ø)	⬆️
nipype/interfaces/base/core.py	86.99% <0%> (+0.19%)	⬆️
nipype/pipeline/engine/workflows.py	78.95% <0%> (+0.33%)	⬆️
nipype/pipeline/plugins/legacymultiproc.py	68% <0%> (+0.5%)	⬆️
nipype/utils/subprocess.py	94.02% <0%> (+0.74%)	⬆️
nipype/utils/filemanip.py	80.38% <0%> (+0.77%)	⬆️
nipype/interfaces/fsl/base.py	87.77% <0%> (+1.11%)	⬆️
nipype/interfaces/io.py	54.94% <0%> (+1.33%)	⬆️
nipype/pipeline/plugins/multiproc.py	84.56% <0%> (+1.85%)	⬆️
nipype/interfaces/fsl/preprocess.py	82.04% <0%> (+2.24%)	⬆️
... and 5 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 154ea61...845463d. Read the comment docs.

[ostanley]

Please let me know if you want any other changes.

[effigies]

This LGTM. One style nitpick and I think this will be good to go.

[effigies]

The failing tests are due to an issue fixed in #2963. Can you merge master or rebase onto master?

[effigies]

Thanks! Merging.