Skip to content

[ENH] Remove warnings in tSNR calculation #1234

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Feb 3, 2016
Merged

[ENH] Remove warnings in tSNR calculation #1234

merged 4 commits into from
Feb 3, 2016

Conversation

oesteban
Copy link
Contributor

@oesteban oesteban commented Oct 5, 2015

Also simplified the interface using default inputs.

satra
satra reviewed Oct 5, 2015
View reviewed changes
nipype/algorithms/misc.py
@@ -262,6 +262,11 @@ class TSNRInputSpec(BaseInterfaceInputSpec):
in_file = InputMultiPath(File(exists=True), mandatory=True,
desc='realigned 4D file or a list of 3D files')
regress_poly = traits.Range(low=1, desc='Remove polynomials')
tsnr_file = File('tsnr.nii.gz', usedefault=True, desc='output tSNR file')
Copy link
Member

Choose a reason for hiding this comment

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

since these are all output_files, we should add hash_files=False

Copy link
Member

Choose a reason for hiding this comment

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

actually could we use name_source and name_template here? i'm not sure though whether they work for all types of interfaces. hopefully then do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, name_source & co. are only defined for CommandLine

@satra
Copy link
Member

satra commented Oct 6, 2015

by making things default and not being able to use name_source we loose the ability to name things based on the input. which means if multiple files are passed in, nipype will create the same named file for each one. i think we need to allow for different names here. so why don't we hold off on this till we move name_source and name_template one level up.

@oesteban oesteban mentioned this pull request Oct 9, 2015
Closed
7 tasks
@chrisgorgo
Copy link
Member

please rebase/merge master - we have fixed the CircleCI tests

@oesteban
Copy link
Contributor Author

Ok, I'll remove the inputs/outputs stuff, so that the default parameters using name_source will be done on #1240 .

@oesteban
Copy link
Contributor Author

oesteban commented Feb 1, 2016

Ok, I have started a list on #1240 with those interfaces that should be revised when name_source works also in BaseInterfaces. I have added TSNR to the list, and I will be updating the list with the candidate interfaces.

I think we should review and merge this for now, and come back to the name_source problem when the implementation is available.

@oesteban
Copy link
Contributor Author

oesteban commented Feb 3, 2016

Good to go?

satra added a commit that referenced this pull request Feb 3, 2016
@satra
Merge pull request #1234 from oesteban/enh/DivisionWarningsTSNR
356b028 
[ENH] Remove warnings in tSNR calculation
@satra satra merged commit 356b028 into nipy:master Feb 3, 2016
@oesteban oesteban deleted the enh/DivisionWarningsTSNR branch February 3, 2016 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
needs-review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@oesteban @satra @chrisgorgo