-
Notifications
You must be signed in to change notification settings - Fork 533
RF: dcm2niix interface #2936
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
RF: dcm2niix interface #2936
Conversation
nipype/interfaces/dcm2nii.py
Outdated
|
||
def _parse_files(self, filenames): | ||
outfiles, bvals, bvecs, bids = [], [], [], [] | ||
outtypes = (".nii", ".nii.gz", ".bval", ".bvec", ".json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@neurolabusc not that we were catching it before, but am I missing any output formats?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The recent release (v1.0.20190410) includes ".nrrd" (-e y -z n
) and ". nhdr"/".raw.gz" (-e y -z y
). This was added for the Slicer team and so you may or may not want to support NRRD format export. There is also a deprecated text output ".txt" (-t y
) which predates BIDS.
Codecov Report
@@ Coverage Diff @@
## master #2936 +/- ##
=========================================
- Coverage 67.51% 64.2% -3.32%
=========================================
Files 344 342 -2
Lines 44094 44030 -64
Branches 5556 5552 -4
=========================================
- Hits 29772 28270 -1502
- Misses 13576 14642 +1066
- Partials 746 1118 +372
Continue to review full report at Codecov.
|
@mgxd Any chance of a test? This is pretty tough to evaluate without seeing the code paths exercised. |
we already have a regression test
though I'm not sure it's actually being hit in the current testing. I'll fix this up for the next release |
CircleCI (both py36 and py27):
Travis (py37):
|
looks like we never got around to adding it to testing in #2498 - looks like the original problem (#2498 (comment)) was fixed so I'll add that change here. |
@mgxd I think you need to merge/rebase master to fix the Docker builds. |
e28cd05
to
83ca7b5
Compare
@mgxd What's the status of this one? |
ready for review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable, and the tests are passing. Codecov is flaking out, so unclear what the coverage is. :-/
Summary
This PR fixes the convoluted approach used for gathering
dcm2niix
outputs.List of changes proposed in this PR (pull-request)
Acknowledgment