Skip to content

Conversation

oesteban
Copy link
Member

After nipy/nipype#3210 the niworkflows/interfaces/ants.py can go in whole.

@mgxd we need to cut an RC (talking about fMRIPrep) and exercise this particular change in a good range of datasets to make sure all use-cases are covered. Happy to talk about more targetted tests. This integration could be disastrous in the long term.

@oesteban oesteban requested review from effigies and mgxd April 27, 2020 16:45
@pull-assistant
Copy link

pull-assistant bot commented Apr 27, 2020

Score: 0.85

Best reviewed: commit by commit


Optimal code review plan (1 warning)

MAINT: Finalize upstreaming of ants interfaces to nipype

niworkflows/interfaces/ants.py 71% changes removed in fix: revise some imp...

     fix: revise some imports from nipype, rework ImageMaths temporary patc...

     chore: pin nipype-1.5.0rc1, which includes the new interfaces

     enh: replace ANTs' ResampleImageBySpacing with RegridToZooms

Powered by Pull Assistant. Last update c53b393 ... c8161b9. Read the comment docs.

@oesteban oesteban force-pushed the enh/finish-ants-upstreaming branch from 52831d2 to b6721a9 Compare April 27, 2020 22:07
@codecov
Copy link

codecov bot commented Apr 27, 2020

Codecov Report

Merging #506 into master will decrease coverage by 4.61%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #506      +/-   ##
==========================================
- Coverage   64.65%   60.03%   -4.62%     
==========================================
  Files          44       43       -1     
  Lines        5375     5170     -205     
  Branches      786      749      -37     
==========================================
- Hits         3475     3104     -371     
- Misses       1742     1931     +189     
+ Partials      158      135      -23     
Flag Coverage Δ
#documentation ?
#masks ?
#reportlettests ?
#travis 60.03% <66.66%> (+0.20%) ⬆️
#unittests ?
Impacted Files Coverage Δ
niworkflows/utils/images.py 80.00% <40.00%> (-2.76%) ⬇️
niworkflows/anat/ants.py 13.01% <54.54%> (-57.77%) ⬇️
niworkflows/func/util.py 22.35% <100.00%> (-57.65%) ⬇️
niworkflows/interfaces/ants.py 100.00% <100.00%> (+34.37%) ⬆️
niworkflows/interfaces/images.py 66.14% <100.00%> (+0.25%) ⬆️
niworkflows/anat/freesurfer.py 39.13% <0.00%> (-52.18%) ⬇️
niworkflows/anat/skullstrip.py 30.00% <0.00%> (-50.00%) ⬇️
niworkflows/interfaces/itk.py 26.92% <0.00%> (-12.31%) ⬇️
... and 8 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 b7f01bb...c8161b9. Read the comment docs.

@oesteban
Copy link
Member Author

With this perspective, this PR can be postponed until after the RC.

@oesteban oesteban added this to the 1.3.0 milestone May 22, 2020
@oesteban oesteban force-pushed the enh/finish-ants-upstreaming branch 2 times, most recently from 043eb57 to 6d96b40 Compare June 4, 2020 00:51
@oesteban oesteban marked this pull request as draft June 4, 2020 00:59
@oesteban oesteban force-pushed the enh/finish-ants-upstreaming branch 3 times, most recently from 08f14af to dbb0bfa Compare June 9, 2020 20:20
@oesteban oesteban force-pushed the enh/finish-ants-upstreaming branch from dbb0bfa to 05ed09a Compare June 12, 2020 22:47
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.

1 participant