Skip to content

FIX: Permit identity transforms in list of transforms given to ants.ApplyTransforms #3237

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 3 commits into from
Aug 15, 2020

Conversation

effigies
Copy link
Member

Summary

antsApplyTransforms currently permits identity or a chain of transformations, but it can be useful to insert an identity into a chain.

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

  • Changes transforms from Either(InputMultiObject(File), "identity") to InputMultiObject(Either(File, "identity")).
  • Change InputMultiPath to InputMultiObject throughout the file, in passing.
  • Simplifies the logic for constructing --transform arguments.

Acknowledgment

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

@effigies effigies changed the title Fix/applytransforms identity FIX: Permit identity transforms in list of transforms given to ants.ApplyTransforms Aug 14, 2020
@effigies effigies requested a review from oesteban August 14, 2020 19:47
@effigies effigies changed the base branch from master to maint/1.5.x August 14, 2020 19:56
@effigies effigies force-pushed the fix/applytransforms_identity branch from 2310ecc to e30d780 Compare August 14, 2020 19:57
@effigies effigies mentioned this pull request Aug 14, 2020
12 tasks
Copy link
Contributor

@oesteban oesteban left a comment

Choose a reason for hiding this comment

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

Looks great

InputMultiPath(File(exists=True)),
"identity",
transforms = InputMultiObject(
traits.Either(File(exists=True), "identity"),
Copy link
Contributor

Choose a reason for hiding this comment

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

In the latest releases of traits they are recommending Union instead of Either. I don't know when Union started to be available or when they started to recommend it though. I'm just writing this down and don't think it requires any particular action in this PR (cc @satra)

@effigies effigies force-pushed the fix/applytransforms_identity branch from e30d780 to c729c1c Compare August 15, 2020 13:29
@codecov
Copy link

codecov bot commented Aug 15, 2020

Codecov Report

Merging #3237 into maint/1.5.x will increase coverage by 0.03%.
The diff coverage is 85.71%.

Impacted file tree graph

@@               Coverage Diff               @@
##           maint/1.5.x    #3237      +/-   ##
===============================================
+ Coverage        64.96%   65.00%   +0.03%     
===============================================
  Files              302      302              
  Lines            39942    39942              
  Branches          5282     5282              
===============================================
+ Hits             25949    25964      +15     
+ Misses           12927    12918       -9     
+ Partials          1066     1060       -6     
Flag Coverage Δ
#unittests 65.00% <85.71%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
nipype/interfaces/ants/resampling.py 84.78% <85.71%> (+1.08%) ⬆️
nipype/pipeline/plugins/base.py 59.72% <0.00%> (+1.64%) ⬆️
nipype/pipeline/plugins/legacymultiproc.py 69.11% <0.00%> (+3.43%) ⬆️

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 02d1500...c729c1c. Read the comment docs.

@effigies
Copy link
Member Author

Okay, tests passing. Thanks for having a look.

@effigies effigies merged commit 87f90dd into nipy:maint/1.5.x Aug 15, 2020
@effigies effigies deleted the fix/applytransforms_identity branch August 15, 2020 14:09
@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