Skip to content

Fix for Eddy-Wrapper does not work properly #769 #785

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 5 commits into from
Feb 5, 2014

Conversation

JensNRAD
Copy link
Contributor

@JensNRAD JensNRAD commented Feb 4, 2014

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 664f6fd on JensNRAD:eddy_fix into b2d3a81 on nipy:master.

@@ -316,7 +316,8 @@ class EddyInputSpec( FSLCommandInputSpec ):


session = File(exists=True, desc='File containing session indices for all volumes in --imain', argstr='--session=%s' )
in_topup = File(exists=True, desc='Base name for output files from topup', argstr='--topup=%s' )
in_topup = traits.Str('topup_basename', desc='Base name for output files from topup',
argstr='--topup=%s', usedefault=False)
Copy link
Member

Choose a reason for hiding this comment

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

@JensNRAD - i believe chris' suggestion was to break this into two inputs
in_topup_fieldcoef = File(exists=True, argstr="--topup=%s", requires=['in_topup_movpar'], ...) and
in_topup_movpar = File(exists=True, requires=['in_topup_fieldcoef'], ...) note the lack of argstr here.

and then overwrite format_args

if name == 'in_topup_fieldcoeff`:
    return spec.argstr % value.split('_fieldcoef')[0]

@JensNRAD
Copy link
Contributor Author

JensNRAD commented Feb 4, 2014

@satra Hi I changed it according to your suggestion.

satra and others added 2 commits February 4, 2014 13:43
a workflow fix, auto tests and some PEP8 changes
@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0836459 on JensNRAD:eddy_fix into b2d3a81 on nipy:master.

@satra
Copy link
Member

satra commented Feb 4, 2014

@JensNRAD - do these fixes work for you? i actually did them blindly without testing.

@JensNRAD
Copy link
Contributor Author

JensNRAD commented Feb 5, 2014

As far as I can tell the only difference you introduced is the copyfile=False setting (and the adjusted test file), right? It still works, as before, generating this warning

[...]/nipype/nipype/interfaces/base.py:376: UserWarning: Input in_topup_fieldcoef requires inputs: in_topup_movpar
  warn(msg)

@satra
Copy link
Member

satra commented Feb 5, 2014

that warning is fine - it's because of the requires bit.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 0836459 on JensNRAD:eddy_fix into b2d3a81 on nipy:master.

satra added a commit that referenced this pull request Feb 5, 2014
Fix for Eddy-Wrapper does not work properly #769
@satra satra merged commit 4d14512 into nipy:master Feb 5, 2014
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