From 8a29a9a690b3d4085271d1b8c49b7ea588728f2d Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 13 Sep 2019 15:39:24 -0400 Subject: [PATCH 01/21] main modification to add lowerCase option --- dcm2bids/dcm2bids.py | 4 ++-- dcm2bids/sidecar.py | 20 ++++++++++++++++---- dcm2bids/utils.py | 2 +- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/dcm2bids/dcm2bids.py b/dcm2bids/dcm2bids.py index 0d5fdb14..b1d34ab6 100644 --- a/dcm2bids/dcm2bids.py +++ b/dcm2bids/dcm2bids.py @@ -109,7 +109,8 @@ def run(self): sidecars = sorted(sidecars) parser = SidecarPairing(sidecars, self.config["descriptions"], - self.config.get("searchMethod", DEFAULT.searchMethod)) + self.config.get("searchMethod", DEFAULT.searchMethod), + self.config.get("lowerCase", DEFAULT.lowerCase)) parser.build_graph() parser.build_acquisitions(self.participant) parser.find_runs() @@ -169,4 +170,3 @@ def move(self, acquisition): #just move else: os.rename(srcFile, dstFile) - diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index e5d0cb48..be8591bd 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -92,7 +92,8 @@ class SidecarPairing(object): """ def __init__(self, sidecars, descriptions, - searchMethod=DEFAULT.searchMethod): + searchMethod=DEFAULT.searchMethod, + lowerCase=DEFAULT.lowerCase): self.logger = logging.getLogger(__name__) self._searchMethod = "" @@ -102,6 +103,8 @@ def __init__(self, sidecars, descriptions, self.sidecars = sidecars self.descriptions = descriptions self.searchMethod = searchMethod + self.lowerCase = lowerCase + @property @@ -162,16 +165,24 @@ def isLink(self, data, criteria): def compare(name, pattern): if self.searchMethod == "re": return bool(re.search(pattern, str(name))) - else: return fnmatch(str(name), str(pattern)) - result = [] + for tag, pattern in iteritems(criteria): name = data.get(tag) + if self.lowerCase: + if isinstance(pattern, list): + pattern = [x.lower() for x in pattern] + elif isinstance(pattern, str): + pattern = pattern.lower() + if isinstance(name, list): + if self.lowerCase: + name = [x.lower() for x in name] + try: subResult = [ len(name)==len(pattern), @@ -185,6 +196,8 @@ def compare(name, pattern): result.append(all(subResult)) else: + if self.lowerCase and isinstance(name, str): + name = name.lower() result.append(compare(name, pattern)) return all(result) @@ -260,4 +273,3 @@ def duplicates(seq): for runNum, acqInd in enumerate(dup): runStr = DEFAULT.runTpl.format(runNum+1) self.acquisitions[acqInd].customLabels += runStr - diff --git a/dcm2bids/utils.py b/dcm2bids/utils.py index 4b859b0d..5a895a90 100644 --- a/dcm2bids/utils.py +++ b/dcm2bids/utils.py @@ -34,6 +34,7 @@ class DEFAULT(object): searchMethod = "fnmatch" searchMethodChoices = ["fnmatch", "re"] runTpl = "_run-{:02d}" + lowerCase = False #misc tmpDirName = "tmp_dcm2bids" @@ -107,4 +108,3 @@ def run_shell_command(commandLine): logger = logging.getLogger(__name__) logger.info("Running {}".format(commandLine)) return check_output(shlex.split(commandLine)) - From 9346a3c7ff5bddad4c291b28cd4cbec287cef741 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Thu, 5 Dec 2019 10:36:06 -0500 Subject: [PATCH 02/21] Rename lowerCase option to caseSensitive --- dcm2bids/dcm2bids.py | 1 + dcm2bids/sidecar.py | 29 ++++++++++++++++++++++++----- dcm2bids/utils.py | 2 +- docs/4-advance.md | 7 +++++++ 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/dcm2bids/dcm2bids.py b/dcm2bids/dcm2bids.py index 3d4760fc..b3f8a0b8 100644 --- a/dcm2bids/dcm2bids.py +++ b/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) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index bc6cf0d2..b82c3211 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -86,7 +86,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,8 +97,7 @@ def __init__(self, sidecars, descriptions, searchMethod=DEFAULT.searchMethod): self.sidecars = sidecars self.descriptions = descriptions self.searchMethod = searchMethod - self.lowerCase = lowerCase - + self.caseSensitive = caseSensitive @property def searchMethod(self): @@ -122,6 +122,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 @@ -155,10 +173,11 @@ def isLink(self, data, criteria): def compare(name, pattern): if self.searchMethod == "re": - name = name.lower() - pattern = pattern.lower() return bool(re.search(pattern, str(name))) else: + if not self.caseSensitive: + name = name.lower() + pattern = pattern.lower() return fnmatch(str(name), str(pattern)) result = [] diff --git a/dcm2bids/utils.py b/dcm2bids/utils.py index 78ddc233..a10e1804 100644 --- a/dcm2bids/utils.py +++ b/dcm2bids/utils.py @@ -35,7 +35,7 @@ class DEFAULT(object): searchMethod = "fnmatch" searchMethodChoices = ["fnmatch", "re"] runTpl = "_run-{:02d}" - lowerCase = False + caseSensitive = False # misc tmpDirName = "tmp_dcm2bids" diff --git a/docs/4-advance.md b/docs/4-advance.md index 098d6cc3..64afc671 100644 --- a/docs/4-advance.md +++ b/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,12 @@ 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": "false"` + +If true, comparaisons between strings/list will be case sensitive. False is the default behavior. + ## defaceTpl default: `"defaceTpl": None` From 16ab4d262437f54218fc16debde333bcebe60c29 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Thu, 5 Dec 2019 10:43:57 -0500 Subject: [PATCH 03/21] cast name into string --- dcm2bids/sidecar.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index b82c3211..80b2c10a 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -176,9 +176,9 @@ def compare(name, pattern): return bool(re.search(pattern, str(name))) else: if not self.caseSensitive: - name = name.lower() + name = str(name).lower() pattern = pattern.lower() - return fnmatch(str(name), str(pattern)) + return fnmatch(name, pattern) result = [] From 3228a50ebabceaf1fa9ddc6b82c8f9942bec2455 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Thu, 5 Dec 2019 10:46:52 -0500 Subject: [PATCH 04/21] cast pattern into string --- dcm2bids/sidecar.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index 80b2c10a..e030557b 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -177,7 +177,7 @@ def compare(name, pattern): else: if not self.caseSensitive: name = str(name).lower() - pattern = pattern.lower() + pattern = str(pattern).lower() return fnmatch(name, pattern) result = [] From cc298575bf59133f691a16cb1a051af508afa74f Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Tue, 10 Nov 2020 15:56:20 -0500 Subject: [PATCH 05/21] add test for caseSensitive --- .../data/config_test_not_case_sensitive_option.json | 13 +++++++++++++ tests/test_dcm2bids.py | 13 +++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/data/config_test_not_case_sensitive_option.json diff --git a/tests/data/config_test_not_case_sensitive_option.json b/tests/data/config_test_not_case_sensitive_option.json new file mode 100644 index 00000000..ba17a529 --- /dev/null +++ b/tests/data/config_test_not_case_sensitive_option.json @@ -0,0 +1,13 @@ +{ + "caseSensitive": false, + "searchMethod": "fnmatch", + "descriptions": [ + { + "dataType": "anat", + "modalityLabel": "T1w", + "criteria": { + "SidecarFilename": "*mprage*" + } + } + ] +} diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index 278b8811..97d0e908 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -69,5 +69,18 @@ def test_dcm2bids(): fmapMtimeRerun = os.stat(fmapFile).st_mtime assert fmapMtime == fmapMtimeRerun + # Validate caseSensitive false + shutil.rmtree(tmpSubDir) + 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() + + if os.name != 'nt': bidsDir.cleanup() From 44397d933a949fd8b61cc2e255580f6a7e049347 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Tue, 10 Nov 2020 16:08:06 -0500 Subject: [PATCH 06/21] update gitignore with new test --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index f23b6ace..e76f1311 100644 --- a/.gitignore +++ b/.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/* From 017f9b3039f49d6071ec0fb05a671f4a99f41ed1 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Tue, 10 Nov 2020 16:25:44 -0500 Subject: [PATCH 07/21] caseSensitive for every searchMethod --- dcm2bids/sidecar.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index e030557b..875fefa7 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -172,12 +172,13 @@ def isLink(self, data, criteria): """ def compare(name, pattern): + if not self.caseSensitive: + name = str(name).lower() + pattern = str(pattern).lower() + if self.searchMethod == "re": - return bool(re.search(pattern, str(name))) + return bool(re.search(pattern, name)) else: - if not self.caseSensitive: - name = str(name).lower() - pattern = str(pattern).lower() return fnmatch(name, pattern) result = [] From 81de8ad803c4c56614a43c64b86afca418056a68 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Wed, 18 Nov 2020 11:22:11 -0500 Subject: [PATCH 08/21] answer Nick comments - add tests --- docs/4-advance.md | 2 +- ...config_test_not_case_sensitive_option.json | 14 ++++++ tests/test_dcm2bids.py | 48 ++++++++++++++++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/docs/4-advance.md b/docs/4-advance.md index 64afc671..9e11c7a6 100644 --- a/docs/4-advance.md +++ b/docs/4-advance.md @@ -23,7 +23,7 @@ fnmatch is the behaviour (See criteria) by default and the fall back if this opt default: `"caseSensitive": "false"` -If true, comparaisons between strings/list will be case sensitive. False is the default behavior. +If true, comparaisons between strings/list will be case sensitive. ## defaceTpl diff --git a/tests/data/config_test_not_case_sensitive_option.json b/tests/data/config_test_not_case_sensitive_option.json index ba17a529..38aa85c0 100644 --- a/tests/data/config_test_not_case_sensitive_option.json +++ b/tests/data/config_test_not_case_sensitive_option.json @@ -2,12 +2,26 @@ "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" + } } ] } diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index 97d0e908..b6dc847e 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -69,8 +69,14 @@ 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 - shutil.rmtree(tmpSubDir) + bidsDir = TemporaryDirectory() + + tmpSubDir = os.path.join(bidsDir.name, DEFAULT.tmpDirName, "sub-01") shutil.copytree(os.path.join(TEST_DATA_DIR, "sidecars"), tmpSubDir) app = Dcm2bids( @@ -81,6 +87,46 @@ def test_dcm2bids(): bidsDir.name) app.run() + layout = BIDSLayout(bidsDir.name, + validate=False, + ignore='tmp_dcm2bids') + + # Input T1 is UPPER CASE (json) + json_t1 = layout.get(subject='01', + datatype='anat', + extension='json', + suffix='T1w') + + # Input localizer is lowercase (json) + json_localizer = layout.get(subject='01', + extension='json', + suffix='localizer') + + # Asking for something with low and up cases (config file) + json_dwi = layout.get(subject='01', + datatype='dwi', + extension='json', + suffix='dwi') + + 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") + + path_dwi = os.path.join(bidsDir.name, + "sub-01", + "dwi", + "sub-01_dwi.json") + + assert layout.get_subjects() == ["01"] + assert json_t1[0].path == path_t1 + assert json_localizer[0].path == path_localizer + assert json_dwi[0].path == path_dwi if os.name != 'nt': bidsDir.cleanup() From 6ff77f6a19f73d96e099bcdb0ac0efd4b7c4031e Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Mon, 23 Nov 2020 20:29:50 -0500 Subject: [PATCH 09/21] Fix Nick comment --- dcm2bids/sidecar.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index 875fefa7..22074b73 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -183,8 +183,9 @@ def compare(name, pattern): result = [] + for tag, pattern in iteritems(criteria): - name = data.get(tag) + name = data.get(tag) or '' if isinstance(name, list): try: @@ -195,7 +196,6 @@ def compare(name, pattern): subResult = [False] result.append(all(subResult)) - else: result.append(compare(name, pattern)) From 9eda0c16c29c103cf3b10526221c0f4ca57956a8 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Thu, 3 Dec 2020 22:54:27 -0500 Subject: [PATCH 10/21] answer Nick comments --- dcm2bids/sidecar.py | 10 +++--- dcm2bids/utils.py | 2 +- docs/4-advance.md | 5 +-- tests/test_dcm2bids.py | 76 ++++++++++++++++++++++++------------------ 4 files changed, 53 insertions(+), 40 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index 22074b73..1f9f0dad 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -172,13 +172,15 @@ def isLink(self, data, criteria): """ def compare(name, pattern): - if not self.caseSensitive: - name = str(name).lower() - pattern = str(pattern).lower() - if self.searchMethod == "re": return bool(re.search(pattern, name)) else: + name = str(name) + pattern = str(pattern) + if not self.caseSensitive: + name = name.lower() + pattern = pattern.lower() + return fnmatch(name, pattern) result = [] diff --git a/dcm2bids/utils.py b/dcm2bids/utils.py index a10e1804..4fae5c2f 100644 --- a/dcm2bids/utils.py +++ b/dcm2bids/utils.py @@ -35,7 +35,7 @@ class DEFAULT(object): searchMethod = "fnmatch" searchMethodChoices = ["fnmatch", "re"] runTpl = "_run-{:02d}" - caseSensitive = False + caseSensitive = True # misc tmpDirName = "tmp_dcm2bids" diff --git a/docs/4-advance.md b/docs/4-advance.md index 9e11c7a6..d3814e54 100644 --- a/docs/4-advance.md +++ b/docs/4-advance.md @@ -21,9 +21,10 @@ fnmatch is the behaviour (See criteria) by default and the fall back if this opt ## caseSensitive -default: `"caseSensitive": "false"` +default: `"caseSensitive": "true"` -If true, comparaisons between strings/list will be case sensitive. +If false, comparisons between strings/lists will be not case sensitive. +It's only disabled when used with `"searchMethod": "fnmatch"`. ## defaceTpl diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index b6dc847e..d54c654b 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -7,10 +7,22 @@ from bids import BIDSLayout from dcm2bids import Dcm2bids from dcm2bids.utils import DEFAULT, load_json +import 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") @@ -34,12 +46,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") @@ -79,28 +88,46 @@ def test_caseSensitive_false(): 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 = 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_localizer = os.path.join(TEST_DATA_DIR, + "sidecars", + "001_localizer_20100603125600_i00001.json") + # Input T1 is UPPER CASE (json) json_t1 = layout.get(subject='01', datatype='anat', extension='json', suffix='T1w') - # Input localizer is lowercase (json) + # Input localizer is lowercase (json) json_localizer = layout.get(subject='01', extension='json', - suffix='localizer') + suffix='localizer', + run='01') # Asking for something with low and up cases (config file) json_dwi = layout.get(subject='01', @@ -108,25 +135,8 @@ def test_caseSensitive_false(): extension='json', suffix='dwi') - 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") - - path_dwi = os.path.join(bidsDir.name, - "sub-01", - "dwi", - "sub-01_dwi.json") - - assert layout.get_subjects() == ["01"] + assert set(os.listdir(os.path.join(bidsDir.name,'sub-01'))) == {'anat','dwi','localizer'} assert json_t1[0].path == path_t1 assert json_localizer[0].path == path_localizer assert json_dwi[0].path == path_dwi - - if os.name != 'nt': - bidsDir.cleanup() + assert compare_json(original_localizer, json_localizer[0].path) From ed2be9c0dd014250ab205961a3863edc777a0be5 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 4 Dec 2020 10:06:25 -0500 Subject: [PATCH 11/21] add check test order --- tests/test_dcm2bids.py | 55 ++++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 13 deletions(-) diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index d54c654b..dd64d41c 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -1,17 +1,20 @@ # -*- 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 -import 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) @@ -78,8 +81,6 @@ 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 @@ -113,9 +114,17 @@ def test_caseSensitive_false(): "localizer", "sub-01_run-01_localizer.json") - original_localizer = os.path.join(TEST_DATA_DIR, - "sidecars", - "001_localizer_20100603125600_i00001.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', @@ -124,10 +133,20 @@ def test_caseSensitive_false(): suffix='T1w') # Input localizer is lowercase (json) - json_localizer = layout.get(subject='01', - extension='json', - suffix='localizer', - run='01') + 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', @@ -135,8 +154,18 @@ def test_caseSensitive_false(): extension='json', suffix='dwi') - assert set(os.listdir(os.path.join(bidsDir.name,'sub-01'))) == {'anat','dwi','localizer'} + assert set(os.listdir(os.path.join(bidsDir.name, + 'sub-01'))) == {'anat', + 'dwi', + 'localizer'} assert json_t1[0].path == path_t1 - assert json_localizer[0].path == path_localizer + assert json_01_localizer[0].path == path_localizer assert json_dwi[0].path == path_dwi - assert compare_json(original_localizer, json_localizer[0].path) + + # Check order runs + assert compare_json(original_01_localizer, + json_01_localizer[0].path) + assert compare_json(original_02_localizer, + json_02_localizer[0].path) + assert compare_json(original_03_localizer, + json_03_localizer[0].path) From 337455ff7c353b7eb54d5da4ec5e9ff32dfeeccd Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 4 Dec 2020 11:07:49 -0500 Subject: [PATCH 12/21] temp test --- tests/test_dcm2bids.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index dd64d41c..add25aa8 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -24,6 +24,12 @@ def compare_json(original_file, converted_file): converted_json.pop('Dcm2bidsVersion', None) + print(original_file) + print(original_json) + + print(converted_file) + print(converted_json) + return original_json == converted_json From 7e04f15d9c42d9d06bea47c550f1a82f663edd53 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 4 Dec 2020 14:31:21 -0500 Subject: [PATCH 13/21] fix tests --- tests/test_dcm2bids.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index add25aa8..6b7e00f4 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -24,12 +24,6 @@ def compare_json(original_file, converted_file): converted_json.pop('Dcm2bidsVersion', None) - print(original_file) - print(original_json) - - print(converted_file) - print(converted_json) - return original_json == converted_json @@ -168,10 +162,12 @@ def test_caseSensitive_false(): assert json_01_localizer[0].path == path_localizer assert json_dwi[0].path == path_dwi - # Check order runs + # 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_02_localizer[0].path) - assert compare_json(original_03_localizer, json_03_localizer[0].path) + assert compare_json(original_03_localizer, + json_02_localizer[0].path) From bc8c3c01ec381fa5a0da12eaed6ba27e850561dd Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 4 Dec 2020 15:30:12 -0500 Subject: [PATCH 14/21] fix ordering --- dcm2bids/sidecar.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index 1f9f0dad..e4fb4b15 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -37,15 +37,11 @@ def __lt__(self, other): try: lts.append(self.data.get(key) < other.data.get(key)) 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 From 85d8852d7f18d1786f34ee1de67ab967f6074d47 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 4 Dec 2020 15:51:32 -0500 Subject: [PATCH 15/21] test order again --- dcm2bids/sidecar.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index e4fb4b15..c91b28ec 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -33,12 +33,15 @@ def __init__(self, filename, compKeys=DEFAULT.compKeys): def __lt__(self, other): lts = [] + print('self ' + self.data.get('SidecarFilename')) + print('other ' + other.data.get('SidecarFilename')) for key in self.compKeys: try: lts.append(self.data.get(key) < other.data.get(key)) except: lts.append(None) + print(lts) for lt in lts: if lt is not None: return lt From 59a3765f0a0ae83bd27b82eac3faf811dbe869f4 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Fri, 4 Dec 2020 16:14:06 -0500 Subject: [PATCH 16/21] another test --- dcm2bids/dcm2bids.py | 1 - dcm2bids/sidecar.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dcm2bids/dcm2bids.py b/dcm2bids/dcm2bids.py index b3f8a0b8..86c412fc 100644 --- a/dcm2bids/dcm2bids.py +++ b/dcm2bids/dcm2bids.py @@ -124,7 +124,6 @@ def run(self): sidecars.append( Sidecar(filename, self.config.get("compKeys", DEFAULT.compKeys)) ) - sidecars = sorted(sidecars) parser = SidecarPairing( sidecars, diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index c91b28ec..cb1c6e85 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -263,10 +263,11 @@ def duplicates(seq): yield key, locs dstRoots = [_.dstRoot for _ in self.acquisitions] + 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") - + dup = sorted(dup) for runNum, acqInd in enumerate(dup): runStr = DEFAULT.runTpl.format(runNum + 1) self.acquisitions[acqInd].customLabels += runStr From 58067eddbc0e77e1f50270c4c785f17cd8dbd3a4 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Mon, 7 Dec 2020 15:11:40 -0500 Subject: [PATCH 17/21] fir ordering --- dcm2bids/sidecar.py | 13 ++++++++----- tests/test_sidecar.py | 12 ++++++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index cb1c6e85..d5c4bce7 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -33,15 +33,19 @@ def __init__(self, filename, compKeys=DEFAULT.compKeys): def __lt__(self, other): lts = [] - print('self ' + self.data.get('SidecarFilename')) - print('other ' + other.data.get('SidecarFilename')) 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(None) - print(lts) for lt in lts: if lt is not None: return lt @@ -263,7 +267,6 @@ def duplicates(seq): yield key, locs dstRoots = [_.dstRoot for _ in self.acquisitions] - 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") diff --git a/tests/test_sidecar.py b/tests/test_sidecar.py index 7c2857ae..c0ef03e5 100644 --- a/tests/test_sidecar.py +++ b/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] From 09d249e241e0e469961a22af67edae7338d942b4 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Mon, 7 Dec 2020 15:26:13 -0500 Subject: [PATCH 18/21] test dcm_2bids --- tests/test_dcm2bids.py | 4 ++++ tox.ini | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index 6b7e00f4..2e27df10 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -23,7 +23,11 @@ def compare_json(original_file, converted_file): converted_json = json.load(f) converted_json.pop('Dcm2bidsVersion', None) + print('original_json') + print(original_json) + print('converted_json') + print(converted_json) return original_json == converted_json diff --git a/tox.ini b/tox.ini index 1cb1dc46..28ff305d 100644 --- a/tox.ini +++ b/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 From 274ff18da5f3cae8190e62fb4e0c8b772814c66c Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Mon, 7 Dec 2020 15:32:40 -0500 Subject: [PATCH 19/21] add sorted --- dcm2bids/dcm2bids.py | 1 + dcm2bids/sidecar.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/dcm2bids/dcm2bids.py b/dcm2bids/dcm2bids.py index 86c412fc..b3f8a0b8 100644 --- a/dcm2bids/dcm2bids.py +++ b/dcm2bids/dcm2bids.py @@ -124,6 +124,7 @@ def run(self): sidecars.append( Sidecar(filename, self.config.get("compKeys", DEFAULT.compKeys)) ) + sidecars = sorted(sidecars) parser = SidecarPairing( sidecars, diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index d5c4bce7..6562c79f 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -270,7 +270,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") - dup = sorted(dup) for runNum, acqInd in enumerate(dup): runStr = DEFAULT.runTpl.format(runNum + 1) self.acquisitions[acqInd].customLabels += runStr From 770e0fb82d589c41291a588b7579f354c5f5ce40 Mon Sep 17 00:00:00 2001 From: arnaudbore Date: Mon, 7 Dec 2020 15:58:24 -0500 Subject: [PATCH 20/21] remove prints --- dcm2bids/sidecar.py | 1 - tests/test_dcm2bids.py | 4 ---- 2 files changed, 5 deletions(-) diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index 6562c79f..9de683af 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -188,7 +188,6 @@ def compare(name, pattern): result = [] - for tag, pattern in iteritems(criteria): name = data.get(tag) or '' diff --git a/tests/test_dcm2bids.py b/tests/test_dcm2bids.py index 2e27df10..6b7e00f4 100644 --- a/tests/test_dcm2bids.py +++ b/tests/test_dcm2bids.py @@ -23,11 +23,7 @@ def compare_json(original_file, converted_file): converted_json = json.load(f) converted_json.pop('Dcm2bidsVersion', None) - print('original_json') - print(original_json) - print('converted_json') - print(converted_json) return original_json == converted_json From 8ded745d08741dce4dc0f2d25d1d8e8a21c752cb Mon Sep 17 00:00:00 2001 From: Arnaud Bore Date: Mon, 4 Jan 2021 12:29:02 -0500 Subject: [PATCH 21/21] fix get tag + add tag for version 2.1.5 --- CHANGELOG.md | 5 ++++- dcm2bids/sidecar.py | 2 +- dcm2bids/version.py | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c2ed19d..87b0fd7c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/dcm2bids/sidecar.py b/dcm2bids/sidecar.py index 9de683af..73d41bad 100644 --- a/dcm2bids/sidecar.py +++ b/dcm2bids/sidecar.py @@ -189,7 +189,7 @@ def compare(name, pattern): result = [] for tag, pattern in iteritems(criteria): - name = data.get(tag) or '' + name = data.get(tag, '')# or '' if isinstance(name, list): try: diff --git a/dcm2bids/version.py b/dcm2bids/version.py index 8c9bc537..db6ef3bf 100644 --- a/dcm2bids/version.py +++ b/dcm2bids/version.py @@ -3,7 +3,7 @@ """This module take care of the versioning""" # dcm2bids version -__version__ = "2.1.4" +__version__ = "2.1.5" import logging