Skip to content
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

ENH: add interface for AFNI 3dTsmooth #2948

Merged
merged 5 commits into from
Sep 6, 2019
Merged

ENH: add interface for AFNI 3dTsmooth #2948

merged 5 commits into from
Sep 6, 2019

Conversation

gpiantoni
Copy link
Contributor

Summary

add interface for AFNI 3dTsmooth

List of changes proposed in this PR (pull-request)

  • add interface for AFNI 3dTsmooth
  • includes almost all of the options under 3dTsmooth including:
    • Three Point Filtering Options
    • General Linear Filtering Options
    • Adaptive Mean Filtering option

Acknowledgment

  • [x ] (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

effigies
effigies reviewed Jun 26, 2019
View reviewed changes
nipype/interfaces/afni/preprocess.py Outdated
>>> smooth.inputs.in_file = 'functional.nii'
>>> smooth.inputs.adaptive = 5
>>> smooth.cmdline
'3dTsmooth -prefix functional_smooth -adaptive 5 functional.nii'
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'3dTsmooth -prefix functional_smooth -adaptive 5 functional.nii'
'3dTsmooth -adaptive 5 -prefix functional_smooth functional.nii'

@effigies effigies added this to the 1.2.1 milestone Jun 26, 2019
effigies
effigies reviewed Jul 26, 2019
View reviewed changes
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Hi @gpiantoni, do you have a few minutes to finish this up?

nipype/interfaces/afni/preprocess.py Outdated
desc='output file from 3dTSmooth',
argstr='-prefix %s',
name_source='in_file',
genfile=True)
Copy link
Member

Choose a reason for hiding this comment

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

nipype/interfaces/afni/preprocess.py Outdated
name_template='%s_smooth',
desc='output file from 3dTSmooth',
argstr='-prefix %s',
name_source='in_file',
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name_source='in_file',
name_source='in_file')

@effigies effigies modified the milestones: 1.2.1, 1.2.2 Aug 16, 2019
@effigies
Copy link
Member

effigies commented Sep 5, 2019

Hi @gpiantoni, we're planning to have the next release in a little under 3 weeks, if you want to have a shot at finishing this up.

@gpiantoni
Copy link
Contributor Author

I rebased from upstream/master and force-pushed my branch. However, one test fails (because of the change from genfile=True to genfile=None).

I tried to run make check-before-commit locally. This command modifies more than 40 files with very trivial changes. See output of git diff:
nipype

I don't think I'm supposed to commit these changes but I don't know what else to do. Sorry, I don't know how to proceed from here.

@effigies
Copy link
Member

effigies commented Sep 5, 2019

Hi @gpiantoni, I re-ran make specs on the master branch, so if you rebase again, then make specs should only update your test.

@oesteban oesteban modified the milestones: 1.2.2, 1.3.0, 1.2.3 Sep 5, 2019
@codecov
Copy link

codecov bot commented Sep 6, 2019

Codecov Report

Merging #2948 into master will decrease coverage by 0.51%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2948      +/-   ##
==========================================
- Coverage    67.5%   66.99%   -0.52%     
==========================================
  Files         344      343       -1     
  Lines       44028    44027       -1     
  Branches     5551     5548       -3     
==========================================
- Hits        29723    29496     -227     
- Misses      13556    13780     +224     
- Partials      749      751       +2
Flag Coverage Δ
#smoketests 50.43% <ø> (ø) ⬆️
#unittests 64.18% <100%> (-0.8%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/afni/__init__.py 100% <ø> (ø) ⬆️
nipype/interfaces/afni/preprocess.py 82.34% <100%> (+0.32%) ⬆️
nipype/interfaces/nilearn.py 40% <0%> (-56.67%) ⬇️
nipype/testing/fixtures.py 77.33% <0%> (-21.34%) ⬇️
nipype/interfaces/freesurfer/base.py 61.01% <0%> (-19.5%) ⬇️
nipype/workflows/smri/freesurfer/autorecon2.py 65.9% <0%> (-9.2%) ⬇️
nipype/workflows/smri/freesurfer/ba_maps.py 91.66% <0%> (-8.34%) ⬇️
nipype/workflows/smri/freesurfer/recon.py 66.66% <0%> (-7.81%) ⬇️
nipype/workflows/smri/freesurfer/autorecon3.py 68.4% <0%> (-6.95%) ⬇️
nipype/workflows/smri/freesurfer/autorecon1.py 41.66% <0%> (-5.1%) ⬇️
... and 11 more

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 64c7b24...8d76fd2. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 6, 2019 •
edited
Loading

Codecov Report

Merging #2948 into master will decrease coverage by 0.51%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2948      +/-   ##
==========================================
- Coverage    67.5%   66.99%   -0.52%     
==========================================
  Files         344      343       -1     
  Lines       44028    44026       -2     
  Branches     5551     5548       -3     
==========================================
- Hits        29723    29495     -228     
- Misses      13556    13780     +224     
- Partials      749      751       +2
Flag Coverage Δ
#smoketests 50.43% <ø> (ø) ⬆️
#unittests 64.18% <100%> (-0.8%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/afni/__init__.py 100% <ø> (ø) ⬆️
nipype/interfaces/afni/preprocess.py 82.34% <100%> (+0.32%) ⬆️
nipype/interfaces/nilearn.py 40% <0%> (-56.67%) ⬇️
nipype/testing/fixtures.py 77.33% <0%> (-21.34%) ⬇️
nipype/interfaces/freesurfer/base.py 61.01% <0%> (-19.5%) ⬇️
nipype/workflows/smri/freesurfer/autorecon2.py 65.9% <0%> (-9.2%) ⬇️
nipype/workflows/smri/freesurfer/ba_maps.py 91.66% <0%> (-8.34%) ⬇️
nipype/workflows/smri/freesurfer/recon.py 66.66% <0%> (-7.81%) ⬇️
nipype/workflows/smri/freesurfer/autorecon3.py 68.4% <0%> (-6.95%) ⬇️
nipype/workflows/smri/freesurfer/autorecon1.py 41.66% <0%> (-5.1%) ⬇️
... and 11 more

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 64c7b24...018cc43. Read the comment docs.

effigies
effigies approved these changes Sep 6, 2019
View reviewed changes
Copy link
Member

@effigies effigies left a comment

Choose a reason for hiding this comment

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

LGTM. Will merge when tests pass.

@effigies
Copy link
Member

effigies commented Sep 6, 2019

Thanks for your patience, @gpiantoni. In it goes.

@effigies effigies merged commit 09d55ec into nipy:master Sep 6, 2019
@oesteban oesteban mentioned this pull request Sep 7, 2019
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@effigies effigies effigies approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.3
Development

Successfully merging this pull request may close these issues.
3 participants
@gpiantoni @effigies @oesteban