Skip to content

[Issue #1272] Input a list of warping regularization parameters to interfaces.spm.NewSegment #1590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 29, 2016

Conversation

mgclark
Copy link
Contributor

@mgclark mgclark commented Aug 25, 2016

Allows the user to submit a list of warping regularization parameters to an spm.NewSegment routine, which appears to be required for some SPM12 workflows. This is a fix for Issue #1272.

@@ -845,6 +845,8 @@ class NewSegmentInputSpec(SPMCommandInputSpec):
desc='mni, eastern, subj, none ')
warping_regularization = traits.Float(field='warp.reg',
desc='Aproximate distance between sampling points.')
warping_regularization_12 = traits.List(traits.Float(), minlen=5, maxlen=5, field='warp.reg',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should likely be either or with warping_regularization. a couple of alternatives:

you can make warping regularization use traits.Either(traits.Float, traits.List(traits.Float, minlen, maxlen), desc= ...)

that may keep the interface cleaner. in the description you can add that the list of 5 elements is only supported in SPM 12.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion. Added & committed.

@coveralls
Copy link

coveralls commented Aug 25, 2016

Coverage Status

Coverage increased (+0.03%) to 72.263% when pulling 48c5f33 on mgclark:Issue1226_warpreg into eb8a930 on nipy:master.

traits.List(traits.Float(),
minlen=5, maxlen=5),
field='warp.reg'
desc='Warping regularization \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of the \ you can simply create a tuple

desc=('Warping regularization parameter(s). '
      'Accepts float or list of floats (the '
      'latter being required by SPM12)')

@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage decreased (-0.2%) to 72.021% when pulling c2c5746 on mgclark:Issue1226_warpreg into eb8a930 on nipy:master.

@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage increased (+0.03%) to 72.262% when pulling c2c5746 on mgclark:Issue1226_warpreg into eb8a930 on nipy:master.

@coveralls
Copy link

coveralls commented Aug 26, 2016

Coverage Status

Coverage increased (+0.03%) to 72.262% when pulling c2c5746 on mgclark:Issue1226_warpreg into eb8a930 on nipy:master.

@mgclark
Copy link
Contributor Author

mgclark commented Aug 26, 2016

Still new to the testing infrastructure. Unsure why CircleCI is failing. Looks like lots of isinstance() TypeFailures?

@coveralls
Copy link

coveralls commented Aug 28, 2016

Coverage Status

Coverage remained the same at 72.231% when pulling f9cc3fb on mgclark:Issue1226_warpreg into eb8a930 on nipy:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.231% when pulling f9cc3fb on mgclark:Issue1226_warpreg into eb8a930 on nipy:master.

@mgclark
Copy link
Contributor Author

mgclark commented Aug 29, 2016

That worked! Tests passing.

@satra satra merged commit 0374bac into nipy:master Aug 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants