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

ENH: Add new flags to MRtrix/preprocess.py (DWI2Tensor, MRtransform) #3365

Merged
merged 3 commits into from
Oct 13, 2021

Conversation

alexandreroutier
Copy link
Contributor

Summary

This PR adds some new flags to MRtrix commands.

Please let me known if anything is missing.

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

Acknowledgment

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

@codecov
Copy link

codecov bot commented Aug 8, 2021

Codecov Report

Merging #3365 (a1d56ab) into master (7080ef9) will decrease coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3365      +/-   ##
==========================================
- Coverage   65.11%   64.83%   -0.28%     
==========================================
  Files         307      307              
  Lines       40373    40366       -7     
  Branches     5326     5326              
==========================================
- Hits        26288    26171     -117     
- Misses      13014    13103      +89     
- Partials     1071     1092      +21     
Flag Coverage Δ
unittests 64.83% <100.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
nipype/interfaces/mrtrix/preprocess.py 64.10% <100.00%> (+0.23%) ⬆️
nipype/interfaces/r.py 45.83% <0.00%> (-37.51%) ⬇️
nipype/interfaces/nipy/base.py 80.00% <0.00%> (-20.00%) ⬇️
nipype/testing/utils.py 70.90% <0.00%> (-18.19%) ⬇️
nipype/workflows/__init__.py 88.23% <0.00%> (-11.77%) ⬇️
nipype/interfaces/freesurfer/base.py 65.32% <0.00%> (-11.30%) ⬇️
nipype/info.py 78.12% <0.00%> (-9.38%) ⬇️
nipype/__init__.py 46.80% <0.00%> (-6.39%) ⬇️
nipype/pkg_info.py 75.00% <0.00%> (-6.25%) ⬇️
nipype/interfaces/fsl/base.py 77.65% <0.00%> (-3.20%) ⬇️
... and 28 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 7080ef9...a1d56ab. Read the comment docs.

@alexandreroutier
Copy link
Contributor Author

Hello,

I was wondering if anybody had the chance to look at this PR? :)

I would like to create another PR with some extra flags for other tools and I would like to be sure that I followed correctly the guidelines.

Best,
Alexandre

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. Apologies for the slow response.

linear_transform = File(
exists=True,
argstr="-linear %s",
position=1,
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused as to why all of these are position=1, but I don't think it can hurt for another one to be.

@effigies effigies merged commit 922db8e into nipy:master Oct 13, 2021
@alexandreroutier alexandreroutier deleted the mrtrix_preprocess branch October 22, 2021 05:58
alexandreroutier added a commit to alexandreroutier/clinica that referenced this pull request Oct 25, 2021
The missing flags for DWI2Tensor and MRTransform interfaces were added in nipy/nipype#3365 and released in nipype 1.7.0. As a result, we do not need the local preprocess.py anymore.
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