fixes afni CenterMass bug

[salma1601]

fixes #2213

[codecov-io]

Codecov Report

Merging #2214 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #2214   +/-   ##
=======================================
  Coverage   72.29%   72.29%           
=======================================
  Files        1178     1178           
  Lines       58736    58736           
  Branches     8456     8456           
=======================================
  Hits        42466    42466           
  Misses      14898    14898           
  Partials     1372     1372

Flag	Coverage Δ
#smoketests	72.29% <0%> (ø)	⬆️
#unittests	69.85% <0%> (ø)	⬆️

Impacted Files	Coverage Δ
nipype/interfaces/afni/utils.py	83.22% <0%> (ø)	⬆️

---

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 4801a4c...5526eed. Read the comment docs.

[codecov-io]

Codecov Report

❗ No coverage uploaded for pull request base (master@6cda3ef). Click here to learn what that means.
The diff coverage is 33.33%.

@@           Coverage Diff            @@
##             master   #2214   +/-   ##
========================================
  Coverage          ?   72.3%           
========================================
  Files             ?    1178           
  Lines             ?   58766           
  Branches          ?    8457           
========================================
  Hits              ?   42492           
  Misses            ?   14901           
  Partials          ?    1373

Flag	Coverage Δ
#smoketests	72.3% <33.33%> (?)
#unittests	69.86% <33.33%> (?)

Impacted Files	Coverage Δ
nipype/interfaces/afni/utils.py	83.5% <33.33%> (ø)

---

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 6cda3ef...31b4927. Read the comment docs.

[effigies]

Does this have any effect on this logic?

nipype/nipype/interfaces/afni/utils.py

Lines 738 to 741 in 5526eed

	if len(sout) > 1:
	outputs['cm'] = [tuple(s) for s in sout]
	else:
	outputs['cm'] = tuple(sout)

In particular I'm wondering whether the second option should now be tuple(sout[0]). I don't have much context here, so I could be entirely wrong.

[salma1601]

In particular I'm wondering whether the second option should now be tuple(sout[0]). I don't have much context here, so I could be entirely wrong.

You are entirely right. In the basic case, only one centre of mass is computed, so there is only one line in the cm_file and the output should be one coordinates tuple. If multiple centres of mass (for instance different ROIs), the output would be a list of tuples.
I am correcting, sorry for that.

[effigies]

This looks reasonable to me. If you wanted to go ahead and make cm always be a list, that would simplify the code. The fact that this is the first time this is coming up suggests nobody's been depending on this single-line case up to now, so it would be a minor breakage.

But this PR fixes the issue without changing the API, so it's 👍 for merge from me.

[salma1601]

The fact that this is the first time this is coming up suggests nobody's been depending on this single-line case up to now, so it would be a minor breakage.

Well actually afni.CenterMass has been merged 6 days ago so may be code simplification is more important than sticking to previous API ?

[effigies]

Cool. Looks good to me.