Skip to content

ENH: Add inputs to mrtrix3.DWIPreprocInputSpec and remove mandatory annotation for pe_dir #3470

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 10 commits into from
Jul 23, 2022
Merged

Conversation

GalKepler
Copy link
Contributor

Summary

Simply removed the "mandatory" tag from pe_dir keyword argument as it isn't really mandatory, for example when using rpe_header (in which dwifslpreproc will read the pe_dir from the associated metadata)

Fixes #3469 .

@codecov
Copy link

codecov bot commented May 11, 2022

Codecov Report

Merging #3470 (43edc43) into master (3b50532) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3470   +/-   ##
=======================================
  Coverage   65.25%   65.25%           
=======================================
  Files         309      309           
  Lines       40855    40858    +3     
  Branches     5379     5379           
=======================================
+ Hits        26659    26662    +3     
  Misses      13122    13122           
  Partials     1074     1074           
Flag Coverage Δ
unittests 65.03% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
nipype/interfaces/mrtrix3/preprocess.py 80.41% <100.00%> (+0.41%) ⬆️

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 3b50532...43edc43. Read the comment docs.

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.

Conventionally, we only use mandatory=True, as mandatory=False is equivalent to not providing it at all. I will commit these changes. @GalBenZvi can you merge master afterwards?

@GalKepler
Copy link
Contributor Author

GalKepler commented Jul 15, 2022

Conventionally, we only use mandatory=True, as mandatory=False is equivalent to not providing it at all. I will commit these changes. @GalBenZvi can you merge master afterwards?

Sure, no problem. (:

@effigies effigies changed the title ENH & FIX: fixed some bugs in the interface to dwifslpreproc and added some more available kwargs ENH: Add inputs to mrtrix3.DWIPreprocInputSpec and remove mandatory annotation for pe_dir Jul 23, 2022
@effigies effigies merged commit 1dee939 into nipy:master Jul 23, 2022
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.

Mrtrix3's DWIPreproc force inputs that are not mandatory
2 participants