-
Notifications
You must be signed in to change notification settings - Fork 532
Fix/dcm2niix #2498
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
Fix/dcm2niix #2498
Changes from 14 commits
6ecd4a9
26d82fe
b60b4d8
6d7bb8d
43c43a7
4c2e885
ec72e99
140b715
e904bbb
7ba24f1
333c5a3
715de9c
dd0503f
c92289b
41c45b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,19 @@ | |
|
||
from ..utils.filemanip import split_filename | ||
from .base import (CommandLine, CommandLineInputSpec, InputMultiPath, traits, | ||
TraitedSpec, OutputMultiPath, isdefined, File, Directory) | ||
TraitedSpec, OutputMultiPath, isdefined, File, Directory, | ||
PackageInfo) | ||
|
||
|
||
class Info(PackageInfo): | ||
"""Handle dcm2niix version information""" | ||
|
||
version_cmd = 'dcm2niix' | ||
|
||
@staticmethod | ||
def parse_version(raw_info): | ||
m = re.search(r'version (\S+)', raw_info) | ||
return m.groups()[0] if m else None | ||
|
||
|
||
class Dcm2niiInputSpec(CommandLineInputSpec): | ||
|
@@ -251,6 +263,8 @@ class Dcm2niixInputSpec(CommandLineInputSpec): | |
argstr="%s", | ||
position=-1, | ||
copyfile=False, | ||
deprecated='1.0.2', | ||
new_name='source_dir', | ||
mandatory=True, | ||
xor=['source_dir']) | ||
source_dir = Directory( | ||
|
@@ -260,16 +274,28 @@ class Dcm2niixInputSpec(CommandLineInputSpec): | |
mandatory=True, | ||
xor=['source_names']) | ||
out_filename = traits.Str( | ||
'%t%p', argstr="-f %s", usedefault=True, desc="Output filename") | ||
argstr="-f %s", | ||
desc="Output filename") | ||
output_dir = Directory( | ||
exists=True, argstr='-o %s', genfile=True, desc="Output directory") | ||
".", | ||
usedefault=True, | ||
exists=True, | ||
argstr='-o %s', | ||
desc="Output directory") | ||
bids_format = traits.Bool( | ||
True, argstr='-b', usedefault=True, desc="Create a BIDS sidecar file") | ||
True, | ||
argstr='-b', | ||
usedefault=True, | ||
desc="Create a BIDS sidecar file") | ||
anon_bids = traits.Bool( | ||
argstr='-ba', | ||
requires=["bids_format"], | ||
desc="Anonymize BIDS") | ||
compress = traits.Enum( | ||
'i', ['y', 'i', 'n'], | ||
'y', 'i', 'n', '3', | ||
argstr='-z %s', | ||
usedefault=True, | ||
desc="Gzip compress images - [y=pigz, i=internal, n=no]") | ||
desc="Gzip compress images - [y=pigz, i=internal, n=no, 3=no,3D]") | ||
merge_imgs = traits.Bool( | ||
False, | ||
argstr='-m', | ||
|
@@ -279,16 +305,39 @@ class Dcm2niixInputSpec(CommandLineInputSpec): | |
False, | ||
argstr='-s', | ||
usedefault=True, | ||
desc="Convert only one image (filename as last input") | ||
desc="Single file mode") | ||
verbose = traits.Bool( | ||
False, argstr='-v', usedefault=True, desc="Verbose output") | ||
False, | ||
argstr='-v', | ||
usedefault=True, | ||
desc="Verbose output") | ||
crop = traits.Bool( | ||
False, argstr='-x', usedefault=True, desc="Crop 3D T1 acquisitions") | ||
False, | ||
argstr='-x', | ||
usedefault=True, | ||
desc="Crop 3D T1 acquisitions") | ||
has_private = traits.Bool( | ||
False, | ||
argstr='-t', | ||
usedefault=True, | ||
desc="Flag if text notes includes private patient details") | ||
compression = traits.Enum( | ||
1, 2, 3, 4, 5, 6, 7, 8, 9, | ||
argstr='-%d', | ||
desc="Gz compression level (1=fastest, 9=smallest)") | ||
comment = traits.Str( | ||
argstr='-c %s', | ||
desc="Comment stored as NIfTI aux_file") | ||
ignore_deriv = traits.Bool( | ||
argstr='-i', | ||
desc="Ignore derived, localizer and 2D images") | ||
series_numbers = InputMultiPath( | ||
traits.Str(), | ||
argstr='-n %s', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to check: If this is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. after checking, it should be the latter, making a fix now There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL, nice! |
||
desc="Selectively convert by series number - can be used up to 16 times") | ||
philips_float = traits.Bool( | ||
argstr='-p', | ||
desc="Philips precise float (not display) scaling") | ||
|
||
|
||
class Dcm2niixOutputSpec(TraitedSpec): | ||
|
@@ -306,27 +355,25 @@ class Dcm2niix(CommandLine): | |
|
||
>>> from nipype.interfaces.dcm2nii import Dcm2niix | ||
>>> converter = Dcm2niix() | ||
>>> converter.inputs.source_names = ['functional_1.dcm', 'functional_2.dcm'] | ||
>>> converter.inputs.compress = 'i' | ||
>>> converter.inputs.single_file = True | ||
>>> converter.inputs.output_dir = '.' | ||
>>> converter.cmdline # doctest: +SKIP | ||
'dcm2niix -b y -z i -x n -t n -m n -f %t%p -o . -s y -v n functional_1.dcm' | ||
|
||
>>> flags = '-'.join([val.strip() + ' ' for val in sorted(' '.join(converter.cmdline.split()[1:-1]).split('-'))]) | ||
>>> flags | ||
' -b y -f %t%p -m n -o . -s y -t n -v n -x n -z i ' | ||
>>> converter.inputs.source_dir = 'dicomdir' | ||
>>> converter.inputs.compression = 5 | ||
>>> converter.inputs.output_dir = 'ds005' | ||
>>> converter.cmdline | ||
'dcm2niix -b y -z y -5 -x n -t n -m n -o ds005 -s n -v n dicomdir' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps add:
after this |
||
""" | ||
|
||
input_spec = Dcm2niixInputSpec | ||
output_spec = Dcm2niixOutputSpec | ||
_cmd = 'dcm2niix' | ||
|
||
@property | ||
def version(self): | ||
return Info.version() | ||
|
||
def _format_arg(self, opt, spec, val): | ||
if opt in [ | ||
'bids_format', 'merge_imgs', 'single_file', 'verbose', 'crop', | ||
'has_private' | ||
]: | ||
bools = ['bids_format', 'merge_imgs', 'single_file', 'verbose', 'crop', | ||
'has_private', 'anon_bids', 'ignore_deriv', 'philips_float'] | ||
if opt in bools: | ||
spec = deepcopy(spec) | ||
if val: | ||
spec.argstr += ' y' | ||
|
@@ -335,17 +382,21 @@ def _format_arg(self, opt, spec, val): | |
val = True | ||
if opt == 'source_names': | ||
return spec.argstr % val[0] | ||
if opt == 'series_numbers': | ||
return ' '.join([spec.argstr % v for v in val]) | ||
return super(Dcm2niix, self)._format_arg(opt, spec, val) | ||
|
||
def _run_interface(self, runtime): | ||
new_runtime = super(Dcm2niix, self)._run_interface(runtime) | ||
# may use return code 1 despite conversion | ||
runtime = super(Dcm2niix, self)._run_interface( | ||
runtime, correct_return_codes=(0, 1, )) | ||
if self.inputs.bids_format: | ||
(self.output_files, self.bvecs, self.bvals, | ||
self.bids) = self._parse_stdout(new_runtime.stdout) | ||
self.bids) = self._parse_stdout(runtime.stdout) | ||
else: | ||
(self.output_files, self.bvecs, self.bvals) = self._parse_stdout( | ||
new_runtime.stdout) | ||
return new_runtime | ||
runtime.stdout) | ||
return runtime | ||
|
||
def _parse_stdout(self, stdout): | ||
files = [] | ||
|
@@ -359,11 +410,7 @@ def _parse_stdout(self, stdout): | |
out_file = None | ||
if line.startswith("Convert "): # output | ||
fname = str(re.search('\S+/\S+', line).group(0)) | ||
if isdefined(self.inputs.output_dir): | ||
output_dir = self.inputs.output_dir | ||
else: | ||
output_dir = self._gen_filename('output_dir') | ||
out_file = os.path.abspath(os.path.join(output_dir, fname)) | ||
out_file = os.path.abspath(fname) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you not want to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if omitted, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, sorry, I misunderstood what was going on here. |
||
# extract bvals | ||
if find_b: | ||
bvecs.append(out_file + ".bvec") | ||
|
@@ -372,16 +419,11 @@ def _parse_stdout(self, stdout): | |
# next scan will have bvals/bvecs | ||
elif 'DTI gradients' in line or 'DTI gradient directions' in line or 'DTI vectors' in line: | ||
find_b = True | ||
else: | ||
pass | ||
if out_file: | ||
if self.inputs.compress == 'n': | ||
files.append(out_file + ".nii") | ||
else: | ||
files.append(out_file + ".nii.gz") | ||
ext = '.nii' if self.inputs.compress == 'n' else '.nii.gz' | ||
files.append(out_file + ext) | ||
if self.inputs.bids_format: | ||
bids.append(out_file + ".json") | ||
continue | ||
skip = False | ||
# just return what was done | ||
if not bids: | ||
|
@@ -397,8 +439,3 @@ def _list_outputs(self): | |
if self.inputs.bids_format: | ||
outputs['bids'] = self.bids | ||
return outputs | ||
|
||
def _gen_filename(self, name): | ||
if name == 'output_dir': | ||
return os.getcwd() | ||
return None |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
import os | ||
import pytest | ||
import shutil | ||
|
||
from nipype.interfaces.dcm2nii import Dcm2niix | ||
no_dcm2niix = not bool(Dcm2niix().version) | ||
no_datalad = False | ||
try: | ||
from datalad import api # to pull and grab data | ||
from datalad.support.exceptions import IncompleteResultsError | ||
except ImportError: | ||
no_datalad = True | ||
|
||
DICOM_DIR = 'http://datasets-tests.datalad.org/dicoms/dcm2niix-tests' | ||
|
||
|
||
def fetch_data(tmpdir, dicoms): | ||
"""Fetches some test DICOMs using datalad""" | ||
data = os.path.join(tmpdir, 'data') | ||
api.install(path=data, source=DICOM_DIR) | ||
data = os.path.join(data, dicoms) | ||
api.get(path=data) | ||
return data | ||
|
||
@pytest.mark.skipif(no_datalad, reason="Datalad required") | ||
@pytest.mark.skipif(no_dcm2niix, reason="Dcm2niix required") | ||
def test_dcm2niix_dwi(tmpdir): | ||
tmpdir.chdir() | ||
try: | ||
datadir = fetch_data(tmpdir.strpath, 'Siemens_Sag_DTI_20160825_145811') | ||
except IncompleteResultsError as exc: | ||
pytest.skip("Failed to fetch test data: %s" % str(exc)) | ||
|
||
def assert_dwi(eg, bids): | ||
"Some assertions we will make" | ||
assert eg.outputs.converted_files | ||
assert eg.outputs.bvals | ||
assert eg.outputs.bvecs | ||
outputs = [y for x,y in eg.outputs.get().items()] | ||
if bids: | ||
# ensure all outputs are of equal lengths | ||
assert len(set(map(len, outputs))) == 1 | ||
else: | ||
assert not eg2.outputs.bids | ||
|
||
dcm = Dcm2niix() | ||
dcm.inputs.source_dir = datadir | ||
dcm.inputs.out_filename = '%u%z' | ||
eg1 = dcm.run() | ||
assert_dwi(eg1, True) | ||
|
||
# now run specifying output directory and removing BIDS option | ||
outdir = tmpdir.mkdir('conversion').strpath | ||
dcm.inputs.output_dir = outdir | ||
dcm.inputs.bids_format = False | ||
eg2 = dcm.run() | ||
assert_dwi(eg2, False) |
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.
I'm not sure why this is being deprecated. Is it no longer available in
dcm2niix
, or is it simply disrecommended? Should we usemax_ver
instead?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.
I would prefer to remove this entirely, as it's misleading to
dcm2niix
's actual usage (I've checked as far back as version28Aug2016
)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.
Okay. Looks like you added this interface in the first place, so nobody else really to consult with. I'm fine with this.