changing _format_arg in WarpImageMultiTransform, should fix #2192 #2229
Conversation
…ts (mostly copy of doctests)
Codecov Report
@@ Coverage Diff @@
## master #2229 +/- ##
==========================================
+ Coverage 72.31% 72.34% +0.02%
==========================================
Files 1181 1181
Lines 58939 58970 +31
Branches 8488 8484 -4
==========================================
+ Hits 42622 42659 +37
+ Misses 14934 14931 -3
+ Partials 1383 1380 -3
Continue to review full report at Codecov.
|
| assert wimt.cmdline == 'WarpImageMultiTransform 3 diffusion_weighted.nii diffusion_weighted_wimt.nii -R functional.nii func2anat_coreg_Affine.txt func2anat_InverseWarp.nii.gz dwi2anat_Warp.nii.gz -i dwi2anat_coreg_Affine.txt' | ||
|
|
||
|
|
||
| @pytest.mark.xfail(reason="dj: should it fail?") |
There was a problem hiding this comment.
@satra - fails, i.e. it doesn't raise any exception.
I found it slightly confusing that i can provide wrong index in invert_affine, e.g. 0 or 3 (if you have only 2 affine files), and i don't get any errors. Was wondering if it would be useful to add an extra check.
There was a problem hiding this comment.
i think the issue is that the description is not very clear.
technically, looking at the code, each affine is given a index. seems like this can cause some mishaps. so yes, raising exceptions when the values of index exceed total number of affines is good.
There was a problem hiding this comment.
I think that description of invert_affine trait is quite good. However IMO it's very hard to guess without reading description or the code that affine files are counted, they are getting another indices, and the new indices start from 1.
Unfortunately changing this concept would make old script fail, so was thinking that at least adding some error with meaningful message might save people some time in case of problems.
There was a problem hiding this comment.
that sounds good, but i think it may be good to add an example to describe this situation clearly in the docstring examples.
There was a problem hiding this comment.
I add a comment to an existing doctest and add a new one, please see if the new one really returns cmd that makes sense
I added a set of simple tests, mostly based on doctest, but just testing a few different scenarios.
Have 2 xfail test for wrong (i.e. out of range)
invert_affine. Should this raise exception?