Skip to content

ENH: Add mrrtrix3.MRResize interface #3031

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 7 commits into from
Sep 16, 2019
Merged

ENH: Add mrrtrix3.MRResize interface #3031

merged 7 commits into from
Sep 16, 2019

Conversation

josephmje
Copy link
Contributor

Summary

Adds a new interface for MRtrix's mrresize command

Splits #3008 into 2 PRs.

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

  • MRResize added

Acknowledgment

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

@josephmje josephmje changed the title add mrresize ENH: Adds a new interface for MRtrix's mrresize command Sep 16, 2019
@effigies effigies added this to the 1.2.3 milestone Sep 16, 2019
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.

Thanks, this looks great! I have a few suggestions.

desc='scale the image resolution by the supplied factor. This can be '
'specified either as a single value to be used for all '
'dimensions, or as a comma-separated list of scale factors for '
'each dimension.',
Copy link
Member

Choose a reason for hiding this comment

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

Again, something like

desc='Scale factors to rescale the image by in each dimension'

josephmje and others added 4 commits September 16, 2019 10:35
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
@effigies
Copy link
Member

Just as a heads up, you can test your doctests locally with:

pytest --doctest-modules nipype/interfaces/mrtrix3/utils.py

I think you're going to need to create new interfaces for each of the three examples, or you're going to get bitten by the xor.

@codecov
Copy link

codecov bot commented Sep 16, 2019

Codecov Report

Merging #3031 into master will increase coverage by 0.23%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3031      +/-   ##
==========================================
+ Coverage    67.5%   67.74%   +0.23%     
==========================================
  Files         344      344              
  Lines       44068    44732     +664     
  Branches     5555     5758     +203     
==========================================
+ Hits        29747    30302     +555     
- Misses      13575    13651      +76     
- Partials      746      779      +33
Flag Coverage Δ
#smoketests 50.29% <ø> (ø) ⬆️
#unittests 65.28% <100%> (+0.32%) ⬆️
Impacted Files Coverage Δ
nipype/interfaces/mrtrix3/__init__.py 100% <ø> (ø) ⬆️
nipype/interfaces/mrtrix3/utils.py 83.7% <100%> (+1.28%) ⬆️
nipype/interfaces/afni/preprocess.py 80.91% <0%> (-1.07%) ⬇️
nipype/utils/config.py 66.41% <0%> (+0.28%) ⬆️
nipype/info.py 94.87% <0%> (+0.5%) ⬆️
nipype/interfaces/fsl/preprocess.py 82.93% <0%> (+0.89%) ⬆️
nipype/algorithms/modelgen.py 73.89% <0%> (+5.79%) ⬆️

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 1689fe5...57e83fd. Read the comment docs.

@josephmje
Copy link
Contributor Author

Just as a heads up, you can test your doctests locally with:

pytest --doctest-modules nipype/interfaces/mrtrix3/utils.py

I think you're going to need to create new interfaces for each of the three examples, or you're going to get bitten by the xor.

Great catch! I was also missing a coma

@effigies effigies changed the title ENH: Adds a new interface for MRtrix's mrresize command ENH: Add mrrtrix3.MRResize interface Sep 16, 2019
@effigies
Copy link
Member

LGTM. Thanks!

@effigies effigies merged commit 154ea61 into nipy:master Sep 16, 2019
@josephmje josephmje deleted the mrtrix branch November 18, 2019 20:29
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