Skip to content

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

Merged
merged 15 commits into from
Mar 23, 2018
125 changes: 81 additions & 44 deletions nipype/interfaces/dcm2nii.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -251,6 +263,8 @@ class Dcm2niixInputSpec(CommandLineInputSpec):
argstr="%s",
position=-1,
copyfile=False,
deprecated='1.0.2',
new_name='source_dir',
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 not sure why this is being deprecated. Is it no longer available in dcm2niix, or is it simply disrecommended? Should we use max_ver instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

usage: dcm2niix [options] <in_folder>

I would prefer to remove this entirely, as it's misleading to dcm2niix's actual usage (I've checked as far back as version 28Aug2016)

Copy link
Member

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.

mandatory=True,
xor=['source_dir'])
source_dir = Directory(
Expand All @@ -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',
Expand All @@ -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',
Copy link
Member

Choose a reason for hiding this comment

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

Just to check: If this is series_numbers = ["a", "b", "c"], is the desired output -n a b c or -n a -n b -n c? Right now it's coded for the former.

Copy link
Member Author

Choose a reason for hiding this comment

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

after checking, it should be the latter, making a fix now

Copy link
Member

Choose a reason for hiding this comment

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

You can just do argstr='-n %s...', rather than add a special case to _format_arg.

Copy link
Member Author

Choose a reason for hiding this comment

The 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):
Expand All @@ -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'
Copy link
Member

Choose a reason for hiding this comment

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

perhaps add:

>>> converter.run() # doctest: +SKIP

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'
Expand All @@ -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 = []
Expand All @@ -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)
Copy link
Member

Choose a reason for hiding this comment

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

Do you not want to use output_dir at all? It no longer seems to have any effect in this interface.

Copy link
Member Author

Choose a reason for hiding this comment

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

if omitted, dcm2niix defaults to save within input folder, so I'd prefer to have it save within the node directory/cwd. The output_dir is parsed from dcm2niix's stdout as fname

Copy link
Member

Choose a reason for hiding this comment

The 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")
Expand All @@ -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:
Expand All @@ -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
18 changes: 13 additions & 5 deletions nipype/interfaces/tests/test_auto_Dcm2niix.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,21 @@

def test_Dcm2niix_inputs():
input_map = dict(
anon_bids=dict(
argstr='-ba',
requires=['bids_format'],
),
args=dict(argstr='%s', ),
bids_format=dict(
argstr='-b',
usedefault=True,
),
comment=dict(argstr='-c %s', ),
compress=dict(
argstr='-z %s',
usedefault=True,
),
compression=dict(argstr='-%s', ),
crop=dict(
argstr='-x',
usedefault=True,
Expand All @@ -26,6 +32,7 @@ def test_Dcm2niix_inputs():
argstr='-t',
usedefault=True,
),
ignore_deriv=dict(argstr='-i', ),
ignore_exception=dict(
deprecated='1.0.0',
nohash=True,
Expand All @@ -35,14 +42,13 @@ def test_Dcm2niix_inputs():
argstr='-m',
usedefault=True,
),
out_filename=dict(
argstr='-f %s',
usedefault=True,
),
out_filename=dict(argstr='-f %s', ),
output_dir=dict(
argstr='-o %s',
genfile=True,
usedefault=True,
),
philips_float=dict(argstr='-p', ),
series_numbers=dict(argstr='-n %s', ),
single_file=dict(
argstr='-s',
usedefault=True,
Expand All @@ -56,7 +62,9 @@ def test_Dcm2niix_inputs():
source_names=dict(
argstr='%s',
copyfile=False,
deprecated='1.0.2',
mandatory=True,
new_name='source_dir',
position=-1,
xor=['source_dir'],
),
Expand Down
57 changes: 57 additions & 0 deletions nipype/interfaces/tests/test_dcm2nii.py
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)