[RTM] Code revision: confounds, utilities, etc. #641
Conversation
oesteban
changed the title
fmriprep/interfaces/confounds.py
Outdated
| acompcor = File(exists=True, mandatory=True, desc='input aCompCorr') | ||
| cos_basis = File(exists=True, mandatory=True, desc='input cosine basis') | ||
| motion = File(exists=True, mandatory=True, desc='input motion parameters') | ||
| aroma = File(exists=True, mandatory=True, desc='input ICA-AROMA') |
There was a problem hiding this comment.
These are not mandatory as presently written. I believe the idea is that the individual inputs can fail or (in the case of aroma, at least) not be run, and the interface will still work.
| @@ -5,7 +5,6 @@ | |||
| fMRI preprocessing workflow | |||
| ===== | |||
| """ | |||
| from __future__ import absolute_import, division, print_function, unicode_literals | |||
There was a problem hiding this comment.
Is the plan to completely discontinue Py2 support? At least a couple months ago, we were accepting patches from users who needed Py2, as long as they didn't complicate the code.
There was a problem hiding this comment.
I would discontinue py2 for good at this point, simplify our lives. But you are right, we haven't talked about this. WDYT @chrisfilo @rwblair ?
| mask_erode_mm = traits.Float(0.0, usedefault=True, | ||
| desc='erode input mask (kernel width in mm)') | ||
| erode_mm = traits.Float(0.0, usedefault=True, | ||
| desc='erode output mask (kernel width in mm)') |
There was a problem hiding this comment.
While we're making this interface, let's put in a probability threshold parameter (default 0.95).
fmriprep/utils/misc.py
Outdated
| @@ -2,52 +2,39 @@ | |||
| # -*- coding: utf-8 -*- | |||
| # emacs: -*- mode: python; py-indent-offset: 4; indent-tabs-mode: nil -*- | |||
| # vi: set ft=python sts=4 ts=4 sw=4 et: | |||
| from __future__ import print_function, division, absolute_import, unicode_literals | |||
| """ | |||
| Miscelaneous utilities | |||
| ... '/path/to/sub-045_ses-retest_T1w.nii.gz'], 'test') | ||
| 'sub-045_ses-test_T1w_test.nii.gz' | ||
|
|
||
| """ | ||
| import os.path as op | ||
| from niworkflows.nipype.utils.filemanip import fname_presuffix, filename_to_list | ||
| return op.basename(fname_presuffix(filename_to_list(in_files)[0], |
There was a problem hiding this comment.
To get the behavior described in your docstring: suffix='_' + suffix
Or update the docstring to pass '_test'.
…ready closed)
oesteban
changed the title
fmriprep/interfaces/fmap.py
Outdated
| for i in range(data2d.shape[0]): | ||
| for j in range(data2d.shape[1]): | ||
| for i in list(range(data2d.shape[0])): | ||
| for j in list(range(data2d.shape[1])): |
There was a problem hiding this comment.
Why list? You can iterate over range.
fmriprep/interfaces/images.py
Outdated
| self._results['out_avg'] = genfname(self.inputs.in_files[0], | ||
| suffix='avg') | ||
| self._results['out_avg'] = fname_presuffix(self.inputs.in_files[0], | ||
| suffix='avg', newpath=os.getcwd()) |
There was a problem hiding this comment.
suffix='_avg', newpath=runtime.cwd
fmriprep/interfaces/images.py
Outdated
| @@ -93,7 +91,8 @@ def _run_interface(self, runtime): | |||
| if sqdata.ndim == 5: | |||
| raise RuntimeError('Input image (%s) is 5D' % in_files[0]) | |||
| else: | |||
| in_files = [genfname(in_files[0], suffix='squeezed')] | |||
| in_files = [fname_presuffix(in_files[0], suffix='squeezed', | |||
| newpath=os.getcwd())] | |||
There was a problem hiding this comment.
suffix='_squeezed', newpath=runtime.cwd
fmriprep/interfaces/itk.py
Outdated
| self._results['out_file'] = genfname( | ||
| self.inputs.in_file, suffix='antswarp') | ||
| self._results['out_file'] = fname_presuffix( | ||
| self.inputs.in_file, suffix='antswarp', newpath=os.getcwd()) |
fmriprep/interfaces/utils.py
Outdated
| erode_mm = traits.Float(0.0, usedefault=True, | ||
| desc='erode output mask (kernel width in mm)') | ||
| pthres = traits.Float(0.95, usedefault=True, | ||
| desc='threshold for the tissue probability maps') |
There was a problem hiding this comment.
I'd prefer this be something like probability_threshold, or prob_thresh or something a little more explicit. The main place we'll be reading it is
node = pe.Node(TPM2ROI(probability_threshold=0.95), ...)
fmriprep/utils/bids.py
Outdated
| RuntimeError: Some participants were not found: 14 | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Seems like more newlines than we need?
|
Testing documentation: https://readthedocs.org/projects/fmriprep/builds/5792532/ |
|
Tests are passing here: https://circleci.com/gh/poldracklab/fmriprep/1006 (the circle hook is showing a branch I created to test that it was building on rtd). Merging in... |
#225, depends on #640