Skip to content

ENH: Allow BIDS-style slice timings to be passed directly to TShift #2608

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 6 commits into from
Jul 24, 2018

Conversation

effigies
Copy link
Member

@effigies effigies commented Jun 5, 2018

This is kind of an upstream from fMRIPrep. It replaces a helper Function interface we use before TShift.

This refines the -tpattern inputs to permit both verifying file existence and directly passing a list of floats, which will create a slice timing file to pass as an argument.

Changes proposed in this pull request

  • Create slice_timing input, mutually exclusive with tpattern, which accepts a file
    • If a list of floats is passed, write a file and pass its name to 3dTshift
  • Change tpattern to an Enum, and list valid strings
    • A genericStr is kept as an option, to avoid breaking code that uses @<fname>, but warns user that they should use slice_timing

@effigies effigies added this to the 1.1.0 milestone Jun 5, 2018
@codecov-io
Copy link

codecov-io commented Jun 5, 2018

Codecov Report

Merging #2608 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2608      +/-   ##
==========================================
+ Coverage   67.59%   67.65%   +0.05%     
==========================================
  Files         340      340              
  Lines       43007    43030      +23     
  Branches     5321     5325       +4     
==========================================
+ Hits        29072    29111      +39     
+ Misses      13236    13218      -18     
- Partials      699      701       +2
Flag Coverage Δ
#smoketests 50.52% <ø> (ø) ⬆️
#unittests 65.11% <100%> (+0.06%) ⬆️
Impacted Files Coverage Δ
nipype/interfaces/afni/preprocess.py 82.01% <100%> (+0.5%) ⬆️
nipype/pipeline/plugins/legacymultiproc.py 61.3% <0%> (+5.35%) ⬆️
nipype/interfaces/afni/base.py 68.54% <0%> (+5.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fa55ae...afecc94. Read the comment docs.

@effigies
Copy link
Member Author

effigies commented Jul 2, 2018

Don't have time to do the proper tests on fmriprep today, so punting for this release.

@effigies effigies force-pushed the enh/tshift_pattern branch from 9162cb9 to 7512be3 Compare July 6, 2018 19:00
@effigies
Copy link
Member Author

Just a note that I'm testing this on ds000030/sub-10159. Should be good to go today. Anybody up for a code review?

@effigies
Copy link
Member Author

Patching this branch and nipreps/fmriprep#1160 into the poldracklab/fmriprep:1.1.2 Docker image produced slice-timing corrected files that differed only in timestamp.

iflogger.warning('Passing a file prefixed by "@" will be deprecated'
'; please use the `slice_timing` input')
elif name == 'slice_timing' and isinstance(value, list):
value = self._write_slice_timing()
Copy link
Member

Choose a reason for hiding this comment

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

if we generate this file, perhaps we should include it in the interface's outputs

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems reasonable.

Copy link
Member

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

looks good to me - one minor suggestion

@effigies effigies force-pushed the enh/tshift_pattern branch from 1376e10 to afecc94 Compare July 23, 2018 20:51
@effigies effigies merged commit 04cfa7a into nipy:master Jul 24, 2018
@effigies effigies deleted the enh/tshift_pattern branch July 24, 2018 12:44
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