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.

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

FIX: ANTs' utilities revision - bug fixes and add more operations to ants.ImageMath #3236

Merged
merged 3 commits into from
Aug 13, 2020

Conversation

oesteban
Copy link
Contributor

@oesteban oesteban commented Aug 12, 2020

These revisions are necessary for the antsBrainExtraction workflow we have in niworkflows (and hence niflows too):

  • Missing ImageMath operations, overwriting the behavior of copy_header if the operation is PadImage (see comment below).
  • Disable the copy_header of ResampleImageBySpacing as the original affine is not valid anymore in the output

These added operations are necessary for the antsBrainExtraction workflow we have in niworkflows (and hence niflows too).
@oesteban oesteban marked this pull request as draft August 12, 2020 09:32
@oesteban
Copy link
Contributor Author

For the PadImage operation we need to work around the copy of the header - the center of the image should not be copied from the input header.

oesteban added a commit to oesteban/niworkflows that referenced this pull request Aug 12, 2020
this commit requires nipy/nipype#3236 to be merged.
@codecov
Copy link

codecov bot commented Aug 12, 2020

Codecov Report

Merging #3236 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3236      +/-   ##
==========================================
- Coverage   65.00%   64.96%   -0.05%     
==========================================
  Files         302      302              
  Lines       39929    39929              
  Branches     5279     5279              
==========================================
- Hits        25956    25939      -17     
- Misses      12913    12925      +12     
- Partials     1060     1065       +5     
Flag Coverage Δ
#unittests 64.96% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
nipype/interfaces/ants/utils.py 89.26% <ø> (ø)
nipype/pipeline/plugins/legacymultiproc.py 65.68% <0.00%> (-5.40%) ⬇️
nipype/pipeline/plugins/base.py 58.08% <0.00%> (-1.65%) ⬇️

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 5116ee2...3587533. Read the comment docs.

@oesteban oesteban changed the title ENH: Add more operations to ants.ImageMath FIX: ANTs' utilities revision - bug fixes and add more operations to ants.ImageMath Aug 12, 2020
@oesteban oesteban marked this pull request as ready for review August 12, 2020 12:01
- Disable ``copy_header`` for the ``PadImage`` operation in ``ImageMath``
- Drop the CopyHeader mixin and update auto test of ResampleImageBySpacing
oesteban added a commit to oesteban/niworkflows that referenced this pull request Aug 12, 2020
this commit requires nipy/nipype#3236 to be merged.
@oesteban oesteban requested a review from effigies August 12, 2020 13:09
@effigies
Copy link
Member

Looks reasonable.

@oesteban oesteban requested a review from mgxd August 13, 2020 07:16
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.

Some alternative suggestions. I prefer RST plain text to Python comments. Added some context about what copy_header does.

nipype/interfaces/ants/utils.py Outdated Show resolved Hide resolved
nipype/interfaces/ants/utils.py Outdated Show resolved Hide resolved
nipype/interfaces/ants/utils.py Outdated Show resolved Hide resolved
@oesteban oesteban merged commit f2a2034 into master Aug 13, 2020
@oesteban oesteban deleted the oesteban-patch-1 branch August 13, 2020 12:38
oesteban added a commit to oesteban/niworkflows that referenced this pull request Aug 13, 2020
this commit requires nipy/nipype#3236 to be merged.
oesteban added a commit to oesteban/niworkflows that referenced this pull request Aug 13, 2020
this commit requires nipy/nipype#3236 to be merged.
@effigies effigies added this to the 1.5.1 milestone Aug 16, 2020
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.

2 participants