merge dev and add tests

Dockerfile

@@ -0,0 +1,57 @@
+# Conda image for installing FSL tools
+FROM continuumio/miniconda3 AS build
+
+# Install FSL tools with conda
+COPY environment.yml /tmp/environment.yml
+RUN conda env create -f /tmp/environment.yml
+
+# Install and use conda-pack
+RUN conda install -c conda-forge conda-pack
+RUN conda-pack -n fsl -o /tmp/env.tar && \
+  mkdir /venv && cd /venv && tar xf /tmp/env.tar && \
+  rm /tmp/env.tar
+RUN /venv/bin/conda-unpack
+
+# Runtime image for executing FSL tools
+FROM debian:stable AS runtime
+
+# Copy the conda env from previous stage
+COPY --from=build /venv /venv
+
+# Point to conda executables
+ENV PATH="/venv/bin:$PATH"
+
+# Set FSL variables
+ENV FSLDIR="/venv" 
+ENV FSLCONFDIR="${FSLDIR}/config"
+ENV FSLOUTPUTTYPE="NIFTI"
+ENV FSLMULTIFILEQUIT="TRUE"
+ENV FSLTCLSH="${FSLDIR}/bin/fsltclsh"
+ENV FSLWISH="${FSLDIR}/bin/fslwish"
+ENV FSLGECUDAQ="cuda.q"
+
+# Update and install some utils
+RUN apt-get -y update && apt-get -y install dc wget npm unzip
+
+# Fetch data
+RUN wget -P ${FSLDIR}/data https://git.fmrib.ox.ac.uk/fsl/data_standard/-/raw/master/MNI152_T1_1mm_brain.nii.gz
+
+# Install bids-validator
+RUN npm install -g bids-validator
+
+# Install dcm2niix
+WORKDIR /
+RUN wget https://github.com/rordenlab/dcm2niix/releases/download/v1.0.20240202/dcm2niix_lnx.zip
+RUN unzip dcm2niix_lnx.zip
+RUN mv dcm2niix /usr/bin/
+
+# Install dcm2bids
+
+WORKDIR /
+ADD . /dcm2bids
+WORKDIR /dcm2bids
+RUN pip install -e .
+
+RUN pip install pydeface
+
+ENTRYPOINT ["dcm2bids"]

dcm2bids/acquisition.py

@@ -32,6 +32,7 @@ def __init__(
         src_sidecar=None,
         sidecar_changes=None,
         bids_uri=None,
+        do_not_reorder_entities=None,
         **kwargs
     ):
         self.logger = logging.getLogger(__name__)
@@ -46,6 +47,7 @@ def __init__(
         self.custom_entities = custom_entities
         self.src_sidecar = src_sidecar
         self.bids_uri = bids_uri
+        self.do_not_reorder_entities = do_not_reorder_entities
 
         if sidecar_changes is None:
             self.sidecar_changes = {}
@@ -159,8 +161,8 @@ def setExtraDstFile(self, new_entities):
         """
         Return:
             The destination filename formatted following
-            the v1.8.0 BIDS entity key table
-            https://bids-specification.readthedocs.io/en/v1.8.0/99-appendices/04-entity-table.html
+            the v1.9.0 BIDS entity key table
+            https://bids-specification.readthedocs.io/en/v1.9.0/99-appendices/04-entity-table.html
         """
 
         if self.custom_entities.strip() == "":
@@ -199,11 +201,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,
@@ -213,8 +218,8 @@ def setDstFile(self):
         """
         Return:
             The destination filename formatted following
-            the v1.8.0 BIDS entity key table
-            https://bids-specification.readthedocs.io/en/v1.8.0/99-appendices/04-entity-table.html
+            the v1.9.0 BIDS entity key table
+            https://bids-specification.readthedocs.io/en/v1.9.0/99-appendices/04-entity-table.html
         """
         current_name = self.participant.prefix + self.build_suffix
         new_name = ''
@@ -244,13 +249,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),
@@ -124,10 +132,12 @@ def run(self):
 
         if self.bids_validate:
             try:
-                self.logger.info(f"Validate if {self.output_dir} is BIDS valid.")
-                self.logger.info("Use bids-validator version: ")
-                run_shell_command(['bids-validator', '-v'])
-                run_shell_command(['bids-validator', self.bids_dir])
+                self.logger.info("BIDS VALIDATION")
+                bids_version = run_shell_command(['bids-validator', '-v'], False)
+                self.logger.info(f"Use bids-validator version: {bids_version.decode()[:-1]}")
+                bids_report = run_shell_command(['bids-validator', self.bids_dir])
+                self.logger.info("Report from bids-validator")
+                self.logger.info(bids_report.decode())
             except Exception:
                 self.logger.error("The bids-validator does not seem to work properly. "
                                   "The bids-validator may not be installed on your "
@@ -187,8 +197,18 @@ def move(self, acq, idList, post_op):
                             else:
                                 cmd = cmd.replace('dst_file', str(dstFile))
 
-                            run_shell_command(cmd.split())
-                            continue
+                            try:
+                                std_out = run_shell_command(cmd.split())
+                                self.logger.debug(f"Log from: {cmd}")
+                                self.logger.debug(std_out.decode())
+                                self.logger.info("")
+                                continue
+                            except Exception:
+                                self.logger.error(
+                                  f"The command post_op: \"{cmd}\" "
+                                  "does not seem to work properly. "
+                                  "Check if it is installed on your "
+                                  "computer.\n")
 
             if ".json" in ext:
                 data = acq.dstSidecarData(idList)

dcm2bids/dcm2niix_gen.py

@@ -142,8 +142,16 @@ def execute(self):
                     shutil.rmtree(self.rm_tmp_dir)
                     self.logger.info("Temporary dicom directory removed.")
 
-                self.logger.debug(f"\n{output}")
-                self.logger.info("Check log file for dcm2niix output\n")
+                if "Warning" in output or "Error" in output:
+                    self.logger.info("Log from dcm2niix execution")
+                    if "Warning" in output:
+                        self.logger.warning(f"{output}")
+                    else:
+                        self.logger.error(f"{output}")
+                else:
+                    self.logger.debug(f"\n{output}")
+                    self.logger.info("Check log file for dcm2niix output\n")
+
 
         else:
             for dicomDir in self.dicom_dirs:

dcm2bids/sidecar.py

@@ -97,6 +97,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,
@@ -111,6 +112,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
@@ -419,6 +421,7 @@ def build_acquisitions(self, participant):
                 acq = Acquisition(participant,
                                   src_sidecar=sidecar,
                                   bids_uri=self.bids_uri,
+                                  do_not_reorder_entities=self.do_not_reorder_entities,
                                   **desc)
                 acq.setDstFile()
 
@@ -439,6 +442,7 @@ def build_acquisitions(self, participant):
                 for desc in valid_descriptions:
                     acq = Acquisition(participant,
                                       bids_uri=self.bids_uri,
+                                      do_not_reorder_entities=self.do_not_reorder_entities,
                                       **desc)
                     self.logger.warning(f"    ->  {acq.suffix}")
 
@@ -452,6 +456,7 @@ def searchDcmTagEntity(self, sidecar, desc):
         """
         descWithTask = desc.copy()
         concatenated_matches = {}
+        keys_custom_entities = []
         entities = []
         if "custom_entities" in desc.keys() or self.auto_extract_entities:
             if 'custom_entities' in desc.keys():
@@ -460,9 +465,12 @@ def searchDcmTagEntity(self, sidecar, desc):
             else:
                 descWithTask["custom_entities"] = []
 
+            keys_custom_entities = [curr_entity.split('-')[0] for curr_entity in descWithTask["custom_entities"]]
+
             if self.auto_extract_entities:
                 self.extractors = combine_dict_extractors(self.extractors, DEFAULT.auto_extractors)
 
+            # Loop to check if we find self.extractor
             for dcmTag in self.extractors:
                 if dcmTag in sidecar.data.keys():
                     dcmInfo = sidecar.data.get(dcmTag)
@@ -471,31 +479,36 @@ def searchDcmTagEntity(self, sidecar, desc):
                         if not isinstance(dcmInfo, list):
                             if compile_regex.search(str(dcmInfo)) is not None:
                                 concatenated_matches.update(
-                                  compile_regex.search(str(dcmInfo)).groupdict())
+                                    compile_regex.search(str(dcmInfo)).groupdict())
                         else:
                             for curr_dcmInfo in dcmInfo:
                                 if compile_regex.search(curr_dcmInfo) is not None:
                                     concatenated_matches.update(
-                                      compile_regex.search(curr_dcmInfo).groupdict())
+                                        compile_regex.search(curr_dcmInfo).groupdict())
                                     break
 
             # 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
                 complete_entities = [ent for ent in descWithTask["custom_entities"] if '-' in ent]
                 entities = entities.union(set(complete_entities))
-
             if self.auto_extract_entities:
                 auto_acq = '_'.join([descWithTask['datatype'], descWithTask["suffix"]])
                 if auto_acq in DEFAULT.auto_entities:
                     # Check if these auto entities have been found before merging
                     auto_entities = set(concatenated_matches.keys()).intersection(set(DEFAULT.auto_entities[auto_acq]))
+
                     left_auto_entities = auto_entities.symmetric_difference(set(DEFAULT.auto_entities[auto_acq]))
+                    left_auto_entities = left_auto_entities.difference(keys_custom_entities)
+
                     if left_auto_entities:
-                        self.logger.warning(f"{left_auto_entities} have not been found for datatype '{descWithTask['datatype']}' "
+                        self.logger.warning(f"Entities {left_auto_entities} have not been found "
+                                            f"for datatype '{descWithTask['datatype']}' "
                                             f"and suffix '{descWithTask['suffix']}'.")
 
                     entities = list(entities) + list(auto_entities)

dcm2bids/utils/logger.py

@@ -19,7 +19,7 @@ def setup_logging(log_level, log_file=None):
 
     sh = logging.StreamHandler(sys.stdout)
     sh.setLevel(log_level)
-    sh_fmt = logging.Formatter(fmt="%(levelname)-8s| %(message)s")
+    sh_fmt = CustomFormatter(fmt="%(levelname)-8s| %(message)s")
     sh.setFormatter(sh_fmt)
 
     # default formatting is kept for the log file"
@@ -30,3 +30,30 @@ def setup_logging(log_level, log_file=None):
         datefmt="%Y-%m-%d %H:%M:%S",
         handlers=[fh, sh]
     )
+
+
+class CustomFormatter(logging.Formatter):
+    """Logging colored formatter, adapted from https://stackoverflow.com/a/56944256/3638629"""
+
+    grey = '\x1b[38;21m'
+    blue = '\x1b[38;5;39m'
+    yellow = '\x1b[38;5;226m'
+    red = '\x1b[38;5;196m'
+    bold_red = '\x1b[31;1m'
+    reset = '\x1b[0m'
+
+    def __init__(self, fmt):
+        super().__init__()
+        self.fmt = fmt
+        self.FORMATS = {
+            logging.DEBUG: self.grey + self.fmt + self.reset,
+            logging.INFO: self.blue + self.fmt + self.reset,
+            logging.WARNING: self.yellow + self.fmt + self.reset,
+            logging.ERROR: self.red + self.fmt + self.reset,
+            logging.CRITICAL: self.bold_red + self.fmt + self.reset
+        }
+
+    def format(self, record):
+        log_fmt = self.FORMATS.get(record.levelno)
+        formatter = logging.Formatter(log_fmt)
+        return formatter.format(record)