Merge pull request #68 from arnaudbore/addLowerCaseOption

Add caseSensitive option + fix ordering runs

.gitignore

@@ -131,4 +131,5 @@ dmypy.json
 # dcm2bids
 tests/data/*
 !tests/data/config_test.json
+!tests/data/config_test_not_case_sensitive_option.json
 !tests/data/sidecars/*

CHANGELOG.md

@@ -1,6 +1,9 @@
 # CHANGELOG
 
-## 2.1.5
+## 2.1.5 - 2021-01-04
+
+- Add possibility to be not case sensitive
+- Fix issue 34: dcm2bids not ordering runs chronologically 
 
 ## 2.1.4 - 2019-04-04
 

dcm2bids/dcm2bids.py

@@ -130,6 +130,7 @@ def run(self):
             sidecars,
             self.config["descriptions"],
             self.config.get("searchMethod", DEFAULT.searchMethod),
+            self.config.get("caseSensitive", DEFAULT.caseSensitive)
         )
         parser.build_graph()
         parser.build_acquisitions(self.participant)

dcm2bids/sidecar.py

@@ -35,17 +35,20 @@ def __lt__(self, other):
         lts = []
         for key in self.compKeys:
             try:
-                lts.append(self.data.get(key) < other.data.get(key))
+                if all(key in d for d in (self.data, other.data)):
+                    if self.data.get(key) == other.data.get(key):
+                        lts.append(None)
+                    else:
+                        lts.append(self.data.get(key) < other.data.get(key))
+                else:
+                    lts.append(None)
+
             except:
-                lts.append(False)
+                lts.append(None)
 
         for lt in lts:
-            if lt:
-                return True
-            else:
-                pass
-
-        return False
+            if lt is not None:
+                return lt
 
     def __eq__(self, other):
         return self.data == other.data
@@ -86,7 +89,8 @@ class SidecarPairing(object):
         descriptions (list): List of dictionnaries describing acquisitions
     """
 
-    def __init__(self, sidecars, descriptions, searchMethod=DEFAULT.searchMethod):
+    def __init__(self, sidecars, descriptions, searchMethod=DEFAULT.searchMethod,
+                 caseSensitive=DEFAULT.caseSensitive):
         self.logger = logging.getLogger(__name__)
 
         self._searchMethod = ""
@@ -96,6 +100,7 @@ def __init__(self, sidecars, descriptions, searchMethod=DEFAULT.searchMethod):
         self.sidecars = sidecars
         self.descriptions = descriptions
         self.searchMethod = searchMethod
+        self.caseSensitive = caseSensitive
 
     @property
     def searchMethod(self):
@@ -120,6 +125,24 @@ def searchMethod(self, value):
                 "Search methods implemented: %s", DEFAULT.searchMethodChoices
             )
 
+    @property
+    def caseSensitive(self):
+        return self._caseSensitive
+
+    @caseSensitive.setter
+    def caseSensitive(self, value):
+        if isinstance(value, bool):
+            self._caseSensitive = value
+        else:
+            self._caseSensitive = DEFAULT.caseSensitive
+            self.logger.warning("'%s' is not a boolean", value)
+            self.logger.warning(
+                "Falling back to default: %s", DEFAULT.caseSensitive
+            )
+            self.logger.warning(
+                "Search methods implemented: %s", DEFAULT.caseSensitive
+            )
+
     def build_graph(self):
         """
         Test all the possible links between the list of sidecars and the
@@ -153,14 +176,20 @@ def isLink(self, data, criteria):
 
         def compare(name, pattern):
             if self.searchMethod == "re":
-                return bool(re.search(pattern, str(name)))
-
+                return bool(re.search(pattern, name))
             else:
-                return fnmatch(str(name), str(pattern))
+                name = str(name)
+                pattern = str(pattern)
+                if not self.caseSensitive:
+                    name = name.lower()
+                    pattern = pattern.lower()
+
+                return fnmatch(name, pattern)
 
         result = []
+
         for tag, pattern in iteritems(criteria):
-            name = data.get(tag)
+            name = data.get(tag, '')# or ''
 
             if isinstance(name, list):
                 try:
@@ -171,7 +200,6 @@ def compare(name, pattern):
                     subResult = [False]
 
                 result.append(all(subResult))
-
             else:
                 result.append(compare(name, pattern))
 
@@ -241,7 +269,6 @@ def duplicates(seq):
         for dstRoot, dup in duplicates(dstRoots):
             self.logger.info("%s has %s runs", dstRoot, len(dup))
             self.logger.info("Adding 'run' information to the acquisition")
-
             for runNum, acqInd in enumerate(dup):
                 runStr = DEFAULT.runTpl.format(runNum + 1)
                 self.acquisitions[acqInd].customLabels += runStr

dcm2bids/utils.py

@@ -35,6 +35,7 @@ class DEFAULT(object):
     searchMethod = "fnmatch"
     searchMethodChoices = ["fnmatch", "re"]
     runTpl = "_run-{:02d}"
+    caseSensitive = True
 
     # misc
     tmpDirName = "tmp_dcm2bids"

docs/4-advance.md

@@ -5,6 +5,7 @@ These optional configurations could be insert in the configuration file at the s
 ```
 {
     "searchMethod": "fnmatch",
+    "caseSensitive": true,
     "defaceTpl": "pydeface --outfile {dstFile} {srcFile}",
     "description": [
         ...
@@ -18,6 +19,13 @@ default: `"searchMethod": "fnmatch"`
 
 fnmatch is the behaviour (See criteria) by default and the fall back if this option is set incorrectly. `re` is the other choice if you want more flexibility to match criteria.
 
+## caseSensitive
+
+default: `"caseSensitive": "true"`
+
+If false, comparisons between strings/lists will be not case sensitive.
+It's only disabled when used with `"searchMethod": "fnmatch"`.
+
 ## defaceTpl
 
 default: `"defaceTpl": None`

tests/data/config_test_not_case_sensitive_option.json

@@ -0,0 +1,27 @@
+{
+    "caseSensitive": false,
+    "searchMethod": "fnmatch",
+    "descriptions": [
+        {
+          "dataType": "localizer",
+          "modalityLabel": "localizer",
+          "criteria": {
+              "SeriesDescription": "LOCALIZER"
+          }
+        },
+        {
+            "dataType": "anat",
+            "modalityLabel": "T1w",
+            "criteria": {
+                "SidecarFilename": "*mprage*"
+            }
+        },
+        {
+            "dataType": "dwi",
+            "modalityLabel": "dwi",
+            "criteria": {
+                "SeriesDescription": "DtI"
+            }
+        }
+    ]
+}

tests/test_dcm2bids.py

@@ -1,17 +1,32 @@
 # -*- coding: utf-8 -*-
 
 
+import json
 import os
 import shutil
 from tempfile import TemporaryDirectory
+
 from bids import BIDSLayout
+
 from dcm2bids import Dcm2bids
 from dcm2bids.utils import DEFAULT, load_json
 
 
 TEST_DATA_DIR = os.path.join(os.path.dirname(__file__), "data")
 
 
+def compare_json(original_file, converted_file):
+    with open(original_file) as f:
+        original_json = json.load(f)
+
+    with open(converted_file) as f:
+        converted_json = json.load(f)
+
+    converted_json.pop('Dcm2bidsVersion', None)
+
+    return original_json == converted_json
+
+
 def test_dcm2bids():
     # tmpBase = os.path.join(TEST_DATA_DIR, "tmp")
     # bidsDir = TemporaryDirectory(dir=tmpBase)
@@ -34,12 +49,9 @@ def test_dcm2bids():
     assert layout.get_tasks() == ["rest"]
     assert layout.get_runs() == [1, 2, 3]
 
-    app = Dcm2bids(
-        [TEST_DATA_DIR],
-        "01",
-        os.path.join(TEST_DATA_DIR, "config_test.json"),
-        bidsDir.name,
-    )
+    app = Dcm2bids(TEST_DATA_DIR, "01",
+                   os.path.join(TEST_DATA_DIR, "config_test.json"),
+                   bidsDir.name)
     app.run()
 
     fmapFile = os.path.join(bidsDir.name, "sub-01", "fmap", "sub-01_echo-492_fmap.json")
@@ -69,5 +81,93 @@ def test_dcm2bids():
     fmapMtimeRerun = os.stat(fmapFile).st_mtime
     assert fmapMtime == fmapMtimeRerun
 
-    if os.name != 'nt':
-        bidsDir.cleanup()
+
+def test_caseSensitive_false():
+    # Validate caseSensitive false
+    bidsDir = TemporaryDirectory()
+
+    tmpSubDir = os.path.join(bidsDir.name, DEFAULT.tmpDirName, "sub-01")
+    shutil.copytree(os.path.join(TEST_DATA_DIR, "sidecars"), tmpSubDir)
+
+    app = Dcm2bids(TEST_DATA_DIR, "01",
+                   os.path.join(TEST_DATA_DIR,
+                                "config_test_not_case_sensitive_option.json"),
+                   bidsDir.name)
+    app.run()
+
+    layout = BIDSLayout(bidsDir.name,
+                        validate=False,
+                        ignore='tmp_dcm2bids')
+
+    path_dwi = os.path.join(bidsDir.name,
+                            "sub-01",
+                            "dwi",
+                            "sub-01_dwi.json")
+
+    path_t1 = os.path.join(bidsDir.name,
+                           "sub-01",
+                           "anat",
+                           "sub-01_T1w.json")
+
+    path_localizer = os.path.join(bidsDir.name,
+                                  "sub-01",
+                                  "localizer",
+                                  "sub-01_run-01_localizer.json")
+
+    original_01_localizer = os.path.join(TEST_DATA_DIR,
+                                         "sidecars",
+                                         "001_localizer_20100603125600_i00001.json")
+
+    original_02_localizer = os.path.join(TEST_DATA_DIR,
+                                         "sidecars",
+                                         "001_localizer_20100603125600_i00002.json")
+
+    original_03_localizer = os.path.join(TEST_DATA_DIR,
+                                         "sidecars",
+                                         "001_localizer_20100603125600_i00003.json")
+
+    # Input T1 is UPPER CASE (json)
+    json_t1 = layout.get(subject='01',
+                         datatype='anat',
+                         extension='json',
+                         suffix='T1w')
+
+    # Input localizer is lowercase (json)
+    json_01_localizer = layout.get(subject='01',
+                                   extension='json',
+                                   suffix='localizer',
+                                   run='01')
+
+    json_02_localizer = layout.get(subject='01',
+                                   extension='json',
+                                   suffix='localizer',
+                                   run='02')
+
+    json_03_localizer = layout.get(subject='01',
+                                   extension='json',
+                                   suffix='localizer',
+                                   run='03')
+
+    # Asking for something with low and up cases (config file)
+    json_dwi = layout.get(subject='01',
+                          datatype='dwi',
+                          extension='json',
+                          suffix='dwi')
+
+    assert set(os.listdir(os.path.join(bidsDir.name,
+                                       'sub-01'))) == {'anat',
+                                                       'dwi',
+                                                       'localizer'}
+    assert json_t1[0].path == path_t1
+    assert json_01_localizer[0].path == path_localizer
+    assert json_dwi[0].path == path_dwi
+
+    # Check order runs when same number
+    # i00001 no AcquisitionTime
+    # i00002 AcquisitionTime after i00003
+    assert compare_json(original_01_localizer,
+                        json_01_localizer[0].path)
+    assert compare_json(original_02_localizer,
+                        json_03_localizer[0].path)
+    assert compare_json(original_03_localizer,
+                        json_02_localizer[0].path)

tests/test_sidecar.py

@@ -14,9 +14,17 @@ def sidecarFiles():
 
 
 def test_sidecar_lt(sidecarFiles):
+
     sidecarsDcm2bids = sorted([Sidecar(_) for _ in sidecarFiles])
-    sidecarsExpected = [Sidecar(_) for _ in sorted(sidecarFiles)]
+    sidecarsExpectedOnlyFilename = [Sidecar(_) for _ in sorted(sidecarFiles)]
 
+    # 001_localizer_20100603125600_i00001.json
+    # 001_localizer_20100603125600_i00003.json
     assert sidecarsDcm2bids[0] < sidecarsDcm2bids[1]
+
+    # 003_MPRAGE_20100603125600.json
+    # 001_localizer_20100603125600_i00002.json
     assert sidecarsDcm2bids[4] > sidecarsDcm2bids[2]
-    assert sidecarsDcm2bids[3] == sidecarsExpected[3]
+
+    # 001_localizer_20100603125600_i00002.json
+    assert sidecarsDcm2bids[2] == sidecarsExpectedOnlyFilename[1]

tox.ini

@@ -15,7 +15,7 @@ python =
 
 [testenv]
 deps = -rrequirements-test.txt
-commands = pytest --cov --cov-report=xml
+commands = pytest --cov --cov-report=xml -s
 ; commands = pytest --cov --cov-append --black --flake8 --pylint
 ; depends =
 ;     {py35, py36, py37}: clean