Merge pull request #303 from UNFmontreal/allow_disable_reorder

[ENH] Allow disable automatic reorder entities

dcm2bids/acquisition.py

@@ -30,6 +30,7 @@ def __init__(
         id=None,
         src_sidecar=None,
         sidecar_changes=None,
+        do_not_reorder_entities=None,
         **kwargs
     ):
         self.logger = logging.getLogger(__name__)
@@ -43,6 +44,7 @@ def __init__(
         self.suffix = suffix
         self.custom_entities = custom_entities
         self.src_sidecar = src_sidecar
+        self.do_not_reorder_entities = do_not_reorder_entities
 
         if sidecar_changes is None:
             self.sidecar_changes = {}
@@ -196,11 +198,14 @@ def setExtraDstFile(self, new_entities):
                                 "compliant. Make sure you know what "
                                 "you are doing.")
 
-        if current_name != new_name:
-            self.logger.warning(
-                f"""✅ Filename was reordered according to BIDS entity table order:
-                from:   {current_name}
-                to:     {new_name}""")
+        if not self.do_not_reorder_entities:
+            if current_name != new_name:
+                self.logger.warning(
+                    f"""✅ Filename was reordered according to BIDS entity table order:
+                    from:   {current_name}
+                    to:     {new_name}""")
+        else:
+            new_name = current_name
 
         self.extraDstFile = opj(self.participant.directory,
                                 self.datatype,
@@ -241,13 +246,15 @@ def setDstFile(self):
                                 "compliant. Make sure you know what "
                                 "you are doing.")
 
-        if current_name != new_name:
-            self.logger.warning(
-                f"""✅ Filename was reordered according to BIDS entity table order:
-                from:   {current_name}
-                to:     {new_name}""")
+        self.dstFile = current_name
+        if not self.do_not_reorder_entities:
+            if current_name != new_name:
+                self.logger.warning(
+                    f"""✅ Filename was reordered according to BIDS entity table order:
+                    from:   {current_name}
+                    to:     {new_name}""")
+                self.dstFile = new_name
 
-        self.dstFile = new_name
 
     def dstSidecarData(self, idList):
         """

dcm2bids/cli/dcm2bids.py

@@ -47,12 +47,21 @@ def _build_arg_parser():
                    default=DEFAULT.output_dir,
                    help="Output BIDS directory. [%(default)s]")
 
-    p.add_argument("--auto_extract_entities",
+    g = p.add_mutually_exclusive_group()
+    g.add_argument("--auto_extract_entities",
                    action='store_true',
                    help="If set, it will automatically try to extract entity"
                    "information [task, dir, echo] based on the suffix and datatype."
                    " [%(default)s]")
 
+    g.add_argument("--do_not_reorder_entities",
+                   action='store_true',
+                   help="If set, it will not reorder entities according to the relative "
+                        "ordering indicated in the BIDS specification and use the "
+                        "order defined in custom_entities by the user.\n"
+                        "Cannot be used with --auto_extract_entities. "
+                        " [%(default)s]")
+
     p.add_argument("--bids_validate",
                    action='store_true',
                    help="If set, once your conversion is done it "
@@ -120,6 +129,7 @@ def main():
     logger.info(f"config: {os.path.realpath(args.config)}")
     logger.info(f"BIDS directory: {os.path.realpath(args.output_dir)}")
     logger.info(f"Auto extract entities: {args.auto_extract_entities}")
+    logger.info(f"Reorder entities: {not args.do_not_reorder_entities}")
     logger.info(f"Validate BIDS: {args.bids_validate}\n")
 
     app = Dcm2BidsGen(**vars(args)).run()

dcm2bids/dcm2bids_gen.py

@@ -39,7 +39,8 @@ def __init__(
         config,
         output_dir=DEFAULT.output_dir,
         bids_validate=DEFAULT.bids_validate,
-        auto_extract_entities=False,
+        auto_extract_entities=DEFAULT.auto_extract_entities,
+        do_not_reorder_entities = DEFAULT.do_not_reorder_entities,
         session=DEFAULT.session,
         clobber=DEFAULT.clobber,
         force_dcm2bids=DEFAULT.force_dcm2bids,
@@ -55,11 +56,17 @@ def __init__(
         self.clobber = clobber
         self.bids_validate = bids_validate
         self.auto_extract_entities = auto_extract_entities
+        self.do_not_reorder_entities = do_not_reorder_entities
         self.force_dcm2bids = force_dcm2bids
         self.skip_dcm2niix = skip_dcm2niix
         self.logLevel = log_level
         self.logger = logging.getLogger(__name__)
 
+        if self.auto_extract_entities and self.do_not_reorder_entities:
+            raise ValueError("Auto extract entities is set to True and "
+                              "do not reorder entities is set to True. "
+                              "Please choose only one option.")
+
     @property
     def dicom_dirs(self):
         """List of DICOMs directories"""
@@ -99,6 +106,7 @@ def run(self):
             self.config["descriptions"],
             self.config.get("extractors", {}),
             self.auto_extract_entities,
+            self.do_not_reorder_entities,
             self.config.get("search_method", DEFAULT.search_method),
             self.config.get("case_sensitive", DEFAULT.case_sensitive),
             self.config.get("dup_method", DEFAULT.dup_method),

dcm2bids/sidecar.py

@@ -96,6 +96,7 @@ def __init__(self,
                  descriptions,
                  extractors=DEFAULT.extractors,
                  auto_extractor=DEFAULT.auto_extract_entities,
+                 do_not_reorder_entities=DEFAULT.do_not_reorder_entities,
                  search_method=DEFAULT.search_method,
                  case_sensitive=DEFAULT.case_sensitive,
                  dup_method=DEFAULT.dup_method,
@@ -108,6 +109,7 @@ def __init__(self,
         self.acquisitions = []
         self.extractors = extractors
         self.auto_extract_entities = auto_extractor
+        self.do_not_reorder_entities = do_not_reorder_entities
         self.sidecars = sidecars
         self.descriptions = descriptions
         self.search_method = search_method
@@ -393,8 +395,8 @@ def build_acquisitions(self, participant):
             if len(valid_descriptions) == 1:
                 desc = valid_descriptions[0]
                 desc, sidecar = self.searchDcmTagEntity(sidecar, desc)
-
                 acq = Acquisition(participant,
+                                  do_not_reorder_entities=self.do_not_reorder_entities,
                                   src_sidecar=sidecar, **desc)
                 acq.setDstFile()
 
@@ -414,6 +416,7 @@ def build_acquisitions(self, participant):
                 self.logger.warning(f"Several Pairing  <-  {sidecarName}")
                 for desc in valid_descriptions:
                     acq = Acquisition(participant,
+                                      do_not_reorder_entities=self.do_not_reorder_entities,
                                       **desc)
                     self.logger.warning(f"    ->  {acq.suffix}")
 
@@ -460,7 +463,9 @@ def searchDcmTagEntity(self, sidecar, desc):
 
             # Keep entities asked in custom_entities
             # If dir found in custom_entities and concatenated_matches.keys we keep it
-            if "custom_entities" in desc.keys():
+            if "custom_entities" in desc.keys() and not self.auto_extract_entities:
+                entities = desc["custom_entities"]
+            elif "custom_entities" in desc.keys():
                 entities = set(concatenated_matches.keys()).intersection(set(descWithTask["custom_entities"]))
 
                 # custom_entities not a key for extractor or auto_extract_entities

dcm2bids/utils/utils.py

@@ -28,6 +28,7 @@ class DEFAULT(object):
     session = ""  # also Participant object
     bids_validate = False
     auto_extract_entities = False
+    do_not_reorder_entities = False
     clobber = False
     force_dcm2bids = False
     post_op = []

tests/data/config_test_no_reorder.json

@@ -0,0 +1,16 @@
+{
+    "search_method": "fnmatch",
+    "extractors": {"SeriesDescription": ["task-(?P<task>[a-zA-Z0-9]+)"]},
+    "descriptions": [
+        {
+            "id": "func_task-rest",
+            "datatype": "func",
+            "suffix": "bold",
+            "custom_entities": ["acq-highres", "task"],
+            "criteria": {
+                "SeriesDescription": "*bold*",
+                "ImageType": ["ORIG?NAL", "PRIMARY", "M", "ND", "MOSAIC"]
+            }
+        }
+    ]
+}

tests/test_dcm2bids.py

@@ -571,3 +571,24 @@ def test_dcm2bids_multiple_intendedFor():
                                    "bids::" + os.path.join("sub-01",
                                                            "anat",
                                                            "sub-01_T1w.nii")]
+
+
+def test_dcm2bids_no_reorder_entities():
+    bids_dir = TemporaryDirectory()
+
+    tmp_sub_dir = os.path.join(bids_dir.name, DEFAULT.tmp_dir_name, "sub-01")
+    shutil.copytree(os.path.join(TEST_DATA_DIR, "sidecars"), tmp_sub_dir)
+
+    app = Dcm2BidsGen(TEST_DATA_DIR, "01",
+                      os.path.join(TEST_DATA_DIR, "config_test_no_reorder.json"),
+                      bids_dir.name,
+                      do_not_reorder_entities=True,
+                      auto_extract_entities=False)
+
+    app.run()
+
+    # existing field
+    func_json = os.path.join(bids_dir.name, "sub-01",
+                             "func",
+                             "sub-01_acq-highres_task-rest_bold.json")
+    assert os.path.exists(func_json)