diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index a3b2141c3a765d44a3ebda80d743efbece39035d..534acf1c4f32607530f14572b81e59cb09b9f7f9 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -25,6 +25,8 @@ Changed
* The :meth:`.LabelAtlas.get` method has a new ``binary`` flag, allowing
either a binary mask, or a mask with the original label value, to be
returned.
+* The :mod:`.dicom` module has been updated to work with the latest version of
+ ``dcm2niix``.
Deprecated
diff --git a/fsl/data/dicom.py b/fsl/data/dicom.py
index f7c9a97c785dcdcdc4f4ddc57f55d06b58a436a2..d38d2081d744a27c9edd71902fc3f1909dafa6d9 100644
--- a/fsl/data/dicom.py
+++ b/fsl/data/dicom.py
@@ -29,9 +29,12 @@ import os
import os.path as op
import subprocess as sp
import re
+import sys
import glob
import json
+import shlex
import logging
+import binascii
import nibabel as nib
@@ -44,7 +47,16 @@ log = logging.getLogger(__name__)
MIN_DCM2NIIX_VERSION = (1, 0, 2017, 12, 15)
-"""Minimum version of dcm2niix that is required for this module to work. """
+"""Minimum version of ``dcm2niix`` that is required for this module to work.
+"""
+
+
+CRC_DCM2NIIX_VERSION = (1, 0, 2019, 9, 2)
+"""For versions of ``dcm2niix`` orf this version or newer, the ``-n`` flag,
+used to convert a single DICOM series, requires that a CRC checksum
+identifying the series be passed (see the :func:`seriesCRC`
+function). Versions prior to this require the series number to be passed.
+"""
class DicomImage(fslimage.Image):
@@ -80,9 +92,16 @@ class DicomImage(fslimage.Image):
@memoize.memoize
-def enabled():
- """Returns ``True`` if ``dcm2niix`` is present, and recent enough,
- ``False`` otherwise.
+def installedVersion():
+ """Return a tuple describing the version of ``dcm2niix`` that is installed,
+ or ``None`` if dcm2niix cannot be found, or its version not parsed.
+
+ The returned tuple contains the following fields, all integers:
+ - Major version number
+ - Minor version number
+ - Year
+ - Month
+ - Day
"""
cmd = 'dcm2niix -h'
@@ -102,80 +121,120 @@ def enabled():
match = re.match(versionPattern, word)
- if match is None:
- continue
+ if match is not None:
+ return (int(match.group('major')),
+ int(match.group('minor')),
+ int(match.group('year')),
+ int(match.group('month')),
+ int(match.group('day')))
- installedVersion = (
- int(match.group('major')),
- int(match.group('minor')),
- int(match.group('year')),
- int(match.group('month')),
- int(match.group('day')))
+ except Exception as e:
+ log.debug('Error parsing dcm2niix version string: {}'.format(e))
+ return None
- # make sure installed version
- # is equal to or newer than
- # minimum required version
- for iv, mv in zip(installedVersion, MIN_DCM2NIIX_VERSION):
- if iv > mv: return True
- elif iv < mv: return False
- # if we get here, versions are equal
- return True
+def compareVersions(v1, v2):
+ """Compares two ``dcm2niix`` versions ``v1`` and ``v2``. The versions are
+ assumed to be in the format returned by :func:`installedVersion`.
- except Exception as e:
- log.debug('Error parsing dcm2niix version string: {}'.format(e))
+ :returns: - 1 if ``v1`` is newer than ``v2``
+ - -1 if ``v1`` is older than ``v2``
+ - 0 if ``v1`` the same as ``v2``.
+ """
+
+ for iv1, iv2 in zip(v1, v2):
+ if iv1 > iv2: return 1
+ elif iv1 < iv2: return -1
+ return 0
- return False
+
+def enabled():
+ """Returns ``True`` if ``dcm2niix`` is present, and recent enough,
+ ``False`` otherwise.
+ """
+ installed = installedVersion()
+ required = MIN_DCM2NIIX_VERSION
+ return ((installed is not None) and
+ (compareVersions(installed, required) >= 0))
def scanDir(dcmdir):
- """Uses ``dcm2niix`` to scans the given DICOM directory, and returns a
- list of dictionaries, one for each data series that was identified.
- Each dictionary is populated with some basic metadata about the series.
+ """Uses the ``dcm2niix -b o`` option to generate a BIDS sidecar JSON
+ file for each series in the given DICOM directory. Reads them all in,
+ and returns them as a sequence of dicts.
- :arg dcmdir: Directory containing DICOM files.
+ Some additional metadata is added to each dictionary:
+ - ``DicomDir``: The absolute path to ``dcmdir``
- :returns: A list of dictionaries, each containing metadata about
- one DICOM data series.
+ :arg dcmdir: Directory containing DICOM series
+
+ :returns: A list of dicts, each containing the BIDS sidecar JSON
+ metadata for one DICOM series.
"""
if not enabled():
raise RuntimeError('dcm2niix is not available or is too old')
- dcmdir = op.abspath(dcmdir)
- cmd = 'dcm2niix -b o -ba n -f %s -o . {}'.format(dcmdir)
- snumPattern = re.compile('^[0-9]+')
+ dcmdir = op.abspath(dcmdir)
+ cmd = 'dcm2niix -b o -ba n -f %s -o . "{}"'.format(dcmdir)
+ series = []
with tempdir.tempdir() as td:
with open(os.devnull, 'wb') as devnull:
- sp.call(cmd.split(), stdout=devnull, stderr=devnull)
+ sp.call(shlex.split(cmd), stdout=devnull, stderr=devnull)
files = glob.glob(op.join(td, '*.json'))
if len(files) == 0:
return []
- # sort numerically by series number if possible
- try:
- def sortkey(f):
- match = re.match(snumPattern, f)
- snum = int(match.group(0))
- return snum
-
- files = sorted(files, key=sortkey)
-
- except Exception:
- files = sorted(files)
-
- series = []
for fn in files:
with open(fn, 'rt') as f:
- meta = json.load(f)
+ meta = json.load(f)
meta['DicomDir'] = dcmdir
series.append(meta)
- return series
+ # sort by series number
+ def key(s):
+ return s.get('SeriesNumber', sys.maxsize)
+
+ series = list(sorted(series, key=key))
+
+ return series
+
+
+def seriesCRC(series):
+ """Calculate a checksum string of the given DICOM series.
+
+ The returned string is of the form::
+
+ SeriesCRC[.echonumber]
+
+ Where ``SeriesCRC`` is an unsigned integer which is the CRC32
+ checksum of the ``SeriesInstanceUID``, and ``echonumber`` is
+ the ``EchoNumber`` of the series - this is only present for
+ multi-echo data, where the series is from the second or subsequent
+ echos.
+
+ :arg series: Dict containing BIDS metadata about a DICOM series,
+ as returned by :func:`scanDir`.
+
+ :returns: String containing a CRC32 checksum for the series.
+ """
+
+ uid = series.get('SeriesInstanceUID', None)
+ echo = series.get('EchoNumber', None)
+
+ if uid is None:
+ return None
+
+ crc32 = str(binascii.crc32(uid.encode()))
+
+ if echo is not None and echo > 1:
+ crc32 = '{}.{}'.format(crc32, echo)
+
+ return crc32
def loadSeries(series):
@@ -192,15 +251,28 @@ def loadSeries(series):
if not enabled():
raise RuntimeError('dcm2niix is not available or is too old')
- dcmdir = series['DicomDir']
- snum = series['SeriesNumber']
- desc = series['SeriesDescription']
- cmd = 'dcm2niix -b n -f %s -z n -o . -n {} {}'.format(snum, dcmdir)
+ dcmdir = series['DicomDir']
+ snum = series['SeriesNumber']
+ desc = series['SeriesDescription']
+ version = installedVersion()
+
+ # Newer versions of dcm2niix
+ # require a CRC to identify
+ # series
+ if compareVersions(version, CRC_DCM2NIIX_VERSION) >= 0:
+ ident = seriesCRC(series)
+
+ # Older versions require
+ # the series number
+ else:
+ ident = snum
+
+ cmd = 'dcm2niix -b n -f %s -z n -o . -n "{}" "{}"'.format(ident, dcmdir)
with tempdir.tempdir() as td:
with open(os.devnull, 'wb') as devnull:
- sp.call(cmd.split(), stdout=devnull, stderr=devnull)
+ sp.call(shlex.split(cmd), stdout=devnull, stderr=devnull)
files = glob.glob(op.join(td, '{}*.nii'.format(snum)))
images = [nib.load(f, mmap=False) for f in files]
diff --git a/requirements-dev.txt b/requirements-dev.txt
index d559b9585f52e10bcb9cb8c28048f75fb9466974..0077afd5ed1cd129bfea288b879dc45cdb87dc18 100644
--- a/requirements-dev.txt
+++ b/requirements-dev.txt
@@ -1,6 +1,5 @@
sphinx
sphinx_rtd_theme
-mock
coverage
pytest
pytest-cov
diff --git a/tests/test_deprecated.py b/tests/test_deprecated.py
index 307818960cbdac0f67b7c1f11c8de0c44bc9ebf1..ecd03522088a4b27ae69c18f53c62dba55a1745f 100644
--- a/tests/test_deprecated.py
+++ b/tests/test_deprecated.py
@@ -7,12 +7,22 @@ import pytest
import fsl.utils.deprecated as deprecated
-# the line number of the warning is hard coded in
-# the unit tests below. Don't change the line number!
+# these get updated in the relevant functions
+WARNING_LINE_NUMBER = None
+DEPRECATED_LINE_NUMBER = None
+
+def _linenum(pattern):
+ with open(__file__, 'rt') as f:
+ for i, line in enumerate(f.readlines(), 1):
+ if pattern in line:
+ return i
+ return -1
+
+
def emit_warning():
deprecated.warn('blag', vin='1.0.0', rin='2.0.0', msg='yo')
-
-WARNING_LINE_NUMBER = 13
+ global WARNING_LINE_NUMBER
+ WARNING_LINE_NUMBER = _linenum('deprecated.warn(\'blag\'')
@deprecated.deprecated(vin='1.0.0', rin='2.0.0', msg='yo')
@@ -20,9 +30,9 @@ def depfunc():
pass
def call_dep_func():
- depfunc()
-
-DEPRECATED_LINE_NUMBER = 23
+ depfunc() # mark
+ global DEPRECATED_LINE_NUMBER
+ DEPRECATED_LINE_NUMBER = _linenum('depfunc() # mark')
def _check_warning(w, name, lineno):
diff --git a/tests/test_dicom.py b/tests/test_dicom.py
index 70b5117bc20019249f15cb6298b2ab45c95c222c..e3b444c3c6c535416c86c7a690b5e4e37d2c8d9e 100644
--- a/tests/test_dicom.py
+++ b/tests/test_dicom.py
@@ -1,14 +1,21 @@
#!/usr/bin/env python
#
-# test_dicom.py -
-#
-# Author: Paul McCarthy <pauldmccarthy@gmail.com>
+# These tests require an internet connection, and will only work on linux.
#
-import os.path as op
-import tarfile
+import os.path as op
+import os
+import functools as ft
+import subprocess as sp
+import tarfile
+import zipfile
+import random
+import string
+import binascii
+import contextlib
+import urllib.request as request
+from unittest import mock
-import mock
import pytest
import fsl.data.dicom as fsldcm
@@ -21,14 +28,35 @@ datadir = op.join(op.dirname(__file__), 'testdata')
pytestmark = pytest.mark.dicomtest
-def setup_module():
+@contextlib.contextmanager
+def install_dcm2niix(version='1.0.20190902'):
+ filenames = {
+ '1.0.20190902' : 'v1.0.20190902/dcm2niix_lnx.zip',
+ '1.0.20190410' : 'v1.0.20190410/dcm2niix_11-Apr-2019_lnx.zip',
+ '1.0.20181125' : 'v1.0.20181125/dcm2niix_25-Nov-2018_lnx.zip',
+ '1.0.20171017' : 'v1.0.20171017/dcm2niix_18-Oct-2017_lnx.zip',
+ }
+ prefix = 'https://github.com/rordenlab/dcm2niix/releases/download/'
+ url = prefix + filenames[version]
- if not fsldcm.enabled():
- raise RuntimeError('dcm2niix is not present - tests cannot be run')
+ with tempdir.tempdir() as td:
+ request.urlretrieve(url, 'dcm2niix.zip')
+ with zipfile.ZipFile('dcm2niix.zip', 'r') as f:
+ f.extractall('.')
+
+ os.chmod(op.join(td, 'dcm2niix'), 0o755)
+
+ path = op.pathsep.join((op.abspath('.'), os.environ['PATH']))
+
+ with mock.patch.dict('os.environ', {'PATH' : path}):
+ try:
+ yield
+ finally:
+ fsldcm.installedVersion.invalidate()
-def test_disabled():
+def test_disabled():
with mock.patch('fsl.data.dicom.enabled', return_value=False):
with pytest.raises(RuntimeError):
fsldcm.scanDir('.')
@@ -36,20 +64,37 @@ def test_disabled():
fsldcm.loadSeries({})
+
+def test_installedVersion():
+ tests = [
+ ('1.0.20190902', (1, 0, 2019, 9, 2)),
+ ('1.0.20181125', (1, 0, 2018, 11, 25)),
+ ('1.0.20171017', (1, 0, 2017, 10, 17))]
+
+ for version, expect in tests:
+ fsldcm.installedVersion.invalidate()
+ with install_dcm2niix(version):
+ got = fsldcm.installedVersion()
+ assert got == expect
+
+
+
def test_enabled():
try:
- fsldcm.enabled.invalidate()
- assert fsldcm.enabled()
- fsldcm.enabled.invalidate()
+ with install_dcm2niix('1.0.20190902'):
+ fsldcm.installedVersion.invalidate()
+ assert fsldcm.enabled()
+
# test dcm2niix not present
with mock.patch('subprocess.check_output',
side_effect=Exception()):
+ fsldcm.installedVersion.invalidate()
assert not fsldcm.enabled()
-
# test presence of different versions
tests = [(b'version v2.1.20191212', True),
+ (b'version v1.0.20190902', True),
(b'version v1.0.20171216', True),
(b'version v1.0.20171215', True),
(b'version v1.0.20171214', False),
@@ -59,19 +104,19 @@ def test_enabled():
(b'version blurgh', False)]
for verstr, expected in tests:
- fsldcm.enabled.invalidate()
+ fsldcm.installedVersion.invalidate()
with mock.patch('subprocess.check_output', return_value=verstr):
assert fsldcm.enabled() == expected
finally:
- fsldcm.enabled.invalidate()
+ fsldcm.installedVersion.invalidate()
def test_scanDir():
- with tempdir.tempdir() as td:
+ with install_dcm2niix():
- series = fsldcm.scanDir(td)
+ series = fsldcm.scanDir('.')
assert len(series) == 0
datafile = op.join(datadir, 'example_dicom.tbz2')
@@ -79,42 +124,69 @@ def test_scanDir():
with tarfile.open(datafile) as f:
f.extractall()
- series = fsldcm.scanDir(td)
- assert len(series) == 3
+ series = fsldcm.scanDir('.')
+ assert len(series) == 2
for s in series:
assert (s['PatientName'] == 'MCCARTHY_PAUL' or
s['PatientName'] == 'MCCARTHY_PAUL_2')
+def test_sersiesCRC():
+ RANDOM = object()
+ tests = [
+ ({'SeriesInstanceUID' : 'hello-world'}, '2983461467'),
+ ({'SeriesInstanceUID' : RANDOM, 'EchoNumber' : 0}, RANDOM),
+ ({'SeriesInstanceUID' : RANDOM, 'EchoNumber' : 1}, RANDOM),
+ ({'SeriesInstanceUID' : RANDOM, 'EchoNumber' : 2}, RANDOM),
+ ({'SeriesInstanceUID' : RANDOM, 'EchoNumber' : 3}, RANDOM),
+ ]
+
+ for series, expect in tests:
+ series = dict(series)
+ if expect is RANDOM:
+ expect = ''.join([random.choice(string.ascii_letters + string.digits)
+ for i in range(30)])
+ series['SeriesInstanceUID'] = expect
+ expect = str(binascii.crc32(expect.encode()))
+ echo = series.get('EchoNumber', 0)
+ if echo > 1:
+ expect += '.{}'.format(echo)
+ assert fsldcm.seriesCRC(series) == expect
+
+
def test_loadSeries():
- with tempdir.tempdir() as td:
+ # test a pre-CRC and a post-CRC version
+ for version in ('1.0.20190410', '1.0.20190902'):
- datafile = op.join(datadir, 'example_dicom.tbz2')
+ with install_dcm2niix(version):
- with tarfile.open(datafile) as f:
- f.extractall()
+ datafile = op.join(datadir, 'example_dicom.tbz2')
+
+ with tarfile.open(datafile) as f:
+ f.extractall()
- series = fsldcm.scanDir(td)
- expShape = (512, 512, 1)
- explens = [1, 2]
+ dcmdir = os.getcwd()
+ series = fsldcm.scanDir(dcmdir)
+ expShape = (512, 512, 1)
+ explens = [1, 1]
- for s, explen in zip(series, explens):
+ for s, explen in zip(series, explens):
- imgs = fsldcm.loadSeries(s)
+ imgs = fsldcm.loadSeries(s)
- assert len(imgs) == explen
+ assert len(imgs) == explen
- for img in imgs:
+ for img in imgs:
- assert img.dicomDir == td
- assert img.shape == expShape
- assert img[:].shape == expShape
- assert img.getMeta('PatientName') == 'MCCARTHY_PAUL' or \
- img.getMeta('PatientName') == 'MCCARTHY_PAUL_2'
- assert 'PatientName' in img.metaKeys()
- assert 'MCCARTHY_PAUL' in img.metaValues() or \
- 'MCCARTHY_PAUL_2' in img.metaValues()
- assert ('PatientName', 'MCCARTHY_PAUL') in img.metaItems() or \
- ('PatientName', 'MCCARTHY_PAUL_2') in img.metaItems()
+ assert img.dicomDir == dcmdir
+ assert img.shape == expShape
+ assert img[:].shape == expShape
+ assert img.getMeta('PatientName') == 'MCCARTHY_PAUL' or \
+ img.getMeta('PatientName') == 'MCCARTHY_PAUL_2'
+ assert 'PatientName' in img.metaKeys()
+ assert 'MCCARTHY_PAUL' in img.metaValues() or \
+ 'MCCARTHY_PAUL_2' in img.metaValues()
+ assert ('PatientName', 'MCCARTHY_PAUL') in img.metaItems() or \
+ ('PatientName', 'MCCARTHY_PAUL_2') in img.metaItems()