WIP: antsRegistrationSyNQuick.sh wrapper #1392
Conversation
There was a problem hiding this comment.
Thanks a lot for this contribution, looks good in general. I left some (mostly stylistic) comments.
Beyond style, there are a couple of things that need attention:
-
It would be necessary to add the auto_test:
make check-before-commitat the root folder of the repo.git add nipype/interfaces/ants/tests/test_auto_RegistrationSynQuick.py
-
Also, a better description of the interface and a doctest are missing.
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
too many empty spaces (only 2 necessary)
| desc='Fixed image or source image or reference image') | ||
| moving_image = InputMultiPath(File(exists=True), mandatory=True, argstr='-m %s', | ||
| desc='Moving image or target image') | ||
| output_prefix = traits.Str("transform", usedefault=True, argstr='-o %s', |
There was a problem hiding this comment.
Please use Str from nipype base. (add from ..base import Str on the top, and then use Str instead of traits.Str here).
| desc="A prefix that is prepended to all output files") | ||
|
|
||
| # todo ANTSCommandInputSpec already has this, but I can't figure out how to set it without defining it again | ||
| num_threads = traits.Int(default_value=1, desc='Number of threads (default = 1)', argstr='-n %d') |
There was a problem hiding this comment.
I'm not 100% sure, but I think you can do traits.Int(1, desc ... here.
| num_threads = traits.Int(default_value=1, desc='Number of threads (default = 1)', argstr='-n %d') | ||
|
|
||
| transform_type = traits.Enum('s', 't', 'r', 'a', 'sr', 'b', 'br', argstr='-t %s', | ||
| desc='transform type\n\ |
There was a problem hiding this comment.
please use block strings:
transform_type = traits.Enum('s', 't', 'r', 'a', 'sr', 'b', 'br', argstr='-t %s',
usedefault=True, desc="""\
transform type -
t: translation
r: rigid
a: rigid + affine
s: rigid + affine + deformable syn (default)
sr: rigid + deformable syn
b: rigid + affine + deformable b-spline syn
br: rigid + deformable b-spline syn""")| br: rigid + deformable b-spline syn', | ||
| usedefault=True) | ||
|
|
||
| use_histogram_matching = traits.Bool(default=False, argstr='-j %s', |
There was a problem hiding this comment.
traits.Bool(False, argstr ...)
additionally: you may remove the _format_arg for this input setting argstr='-j %d'
| """ | ||
| Examples | ||
| -------- | ||
|
|
|
|
||
|
|
||
| class RegistrationSynQuick(ANTSCommand): | ||
| """ |
| output_spec = RegistrationSynQuickOutputSpec | ||
|
|
||
| def _format_arg(self, name, spec, value): | ||
| if name == 'use_histogram_matching': |
There was a problem hiding this comment.
please use argstr='-j %d' and remove this
|
|
||
| elif name == 'precision_type': | ||
| if isdefined(self.inputs.precision_type): | ||
| return spec.argstr % {'float': 'f', 'double': 'd'}[value] |
There was a problem hiding this comment.
return spec.argstr % value[0]
| return spec.argstr % {False: '0', True: '1'}[value] | ||
|
|
||
| elif name == 'precision_type': | ||
| if isdefined(self.inputs.precision_type): |
There was a problem hiding this comment.
this is always True since this trait has usedefault=True
|
@fliem - are you planning to finish this PR. Please let us know if you need help? |
|
Hi @djarecka. I won't have time to work on that. It would be great if you could continue. Thanks. |
|
@fliem - I understand. Thank you for your contribution! |
|
continuation in #2347 |
Hi guys,
I created a wrapper for antsRegistrationSyNQuick.sh. It's my first interface, so it would be great if you had suggestions for improvements (I marked a couple of issues in the code with # todo).
Additionally, I'd like to create a wrapper for antsRegistrationSyN.sh.
antsRegistrationSyNQuick.sh and antsRegistrationSyN.sh do not have completely identical input arguments:
-t (transform type) has different valid values in SyN and SyNQuick.
-r is setting a different variable
SyNQuick: -r: histogram bins for mutual information in SyN stage (default = 32);
Syn: -r: radius for cross correlation metric used during SyN stage (default = 4)
What would be the most elegant way to have interfaces for both? Two separat interfaces?