From 1f5c77a62db4ce39b6ba7407f07df0db9ef43937 Mon Sep 17 00:00:00 2001 From: Samuel Guay Date: Fri, 16 Jun 2023 11:23:07 -0400 Subject: [PATCH 1/2] fix --force + improve overall UX --- dcm2bids/cli/dcm2bids.py | 2 +- dcm2bids/cli/dcm2bids_helper.py | 62 +++++++++++++++++++++++---------- dcm2bids/dcm2bids_gen.py | 8 ++--- dcm2bids/dcm2niix_gen.py | 18 +++++----- dcm2bids/utils/args.py | 35 ++++++++----------- dcm2bids/utils/utils.py | 2 +- 6 files changed, 72 insertions(+), 55 deletions(-) diff --git a/dcm2bids/cli/dcm2bids.py b/dcm2bids/cli/dcm2bids.py index 3b8575aa..3285ec0d 100644 --- a/dcm2bids/cli/dcm2bids.py +++ b/dcm2bids/cli/dcm2bids.py @@ -49,7 +49,7 @@ def _build_arg_parser(): " will check if your output folder is BIDS valid. [%(default)s]\n" f"bids-validator needs to be installed check: {DEFAULT.link_bids_validator}") - p.add_argument("--forceDcm2niix", + p.add_argument("--force_dcm2niix", action="store_true", help="Overwrite previous temporary dcm2niix " "output if it exists.") diff --git a/dcm2bids/cli/dcm2bids_helper.py b/dcm2bids/cli/dcm2bids_helper.py index be5d35b4..3c09681e 100644 --- a/dcm2bids/cli/dcm2bids_helper.py +++ b/dcm2bids/cli/dcm2bids_helper.py @@ -1,14 +1,20 @@ # -*- coding: utf-8 -*- -"""helper module""" - +""" +Converts DICOM files to NIfTI files including their JSON sidecars in a +temporary directory which can be inspected to make a dc2mbids config file. +""" +import logging import argparse +import sys +import os from os.path import join as opj - +from pathlib import Path +from datetime import datetime from dcm2bids.dcm2niix_gen import Dcm2niixGen -from dcm2bids.utils.args import assert_dirs_empty +from dcm2bids.utils.args import add_overwrite_arg, assert_dirs_empty from dcm2bids.utils.utils import DEFAULT - +from dcm2bids.utils.logger import setup_logging def _build_arg_parser(): p = argparse.ArgumentParser(description=__doc__, epilog=DEFAULT.doc, @@ -19,32 +25,52 @@ def _build_arg_parser(): help="DICOM files directory.") p.add_argument("-o", "--output_dir", - required=False, default=DEFAULT.cliOutputDir, - help="Output BIDS directory." - " (Default: %(default)s)") - - p.add_argument('--force', + required=False, + default=Path(DEFAULT.cliOutputDir) / DEFAULT.tmpDirName / DEFAULT.helperDir, + help=f'Output directory. (Default: [%(default)s]') + + p.add_argument("-n", "--nest", + nargs="?", const=True, default=False, required=False, + help=f"Nest a directory in . Useful if many helper runs are needed\n" + f"to make a config file due to slight variations in MRI acquisitions.\n" + f"Defaults to DICOM_DIR if no name is provided.\n" + f"(Default: [%(default)s])") + + p.add_argument('--force', '--force_dcm2niix', dest='overwrite', action='store_true', help='Force command to overwrite existing output files.') return p - def main(): """Let's go""" parser = _build_arg_parser() args = parser.parse_args() + out_dir = Path(args.output_dir) + log_path = Path(DEFAULT.cliOutputDir) / DEFAULT.tmpDirName / "log" / f"helper_{datetime.now().isoformat().replace(':', '')}.log" + if args.nest: + if isinstance(args.nest, str): + log_path = Path(str(log_path).replace("helper_", f"helper_{args.nest.replace(os.path.sep, '-')}_")) + out_dir = out_dir / args.nest + else: + log_path = Path(str(log_path).replace("helper_", f"helper_{args.dicom_dir[0].replace(os.path.sep, '-')}_")) + out_dir = out_dir / args.dicom_dir[0] - out_folder = opj(args.output_dir, 'tmp_dcm2bids', 'helper') - assert_dirs_empty(parser, args, out_folder) + log_path.parent.mkdir(parents=True, exist_ok=True) + setup_logging("info", log_path) + logger = logging.getLogger(__name__) - app = Dcm2niixGen(dicomDirs=args.dicom_dir, bidsDir=args.output_dir) - rsl = app.run() - print("Example in:") - print(opj(args.output_dir, DEFAULT.tmpDirName, DEFAULT.helperDir)) + logger.info("Running the following command: \n" + " ".join(sys.argv) + "\n") - return rsl + assert_dirs_empty(parser, args, out_dir) + + app = Dcm2niixGen(dicomDirs = args.dicom_dir, bidsDir = out_dir, helper = True) + rsl = app.run(force=args.overwrite) + logger.info(f"Helper files in: {out_dir}\n") + logger.info(f"Log file saved at {log_path}") + + return rsl if __name__ == "__main__": main() diff --git a/dcm2bids/dcm2bids_gen.py b/dcm2bids/dcm2bids_gen.py index f45ca3a8..952ea917 100644 --- a/dcm2bids/dcm2bids_gen.py +++ b/dcm2bids/dcm2bids_gen.py @@ -32,7 +32,7 @@ class Dcm2BidsGen(object): output_dir (path): Path to the BIDS base folder session (str): Optional label of a session clobber (boolean): Overwrite file if already in BIDS folder - forceDcm2niix (boolean): Forces a cleaning of a previous execution of + force_dcm2niix (boolean): Forces a cleaning of a previous execution of dcm2niix log_level (str): logging level """ @@ -47,7 +47,7 @@ def __init__( auto_extract_entities=DEFAULT.auto_extract_entities, session=DEFAULT.session, clobber=DEFAULT.clobber, - forceDcm2niix=DEFAULT.forceDcm2niix, + force_dcm2niix=DEFAULT.force_dcm2niix, log_level=DEFAULT.logLevel, **_ ): @@ -60,7 +60,7 @@ def __init__( self.clobber = clobber self.bids_validate = bids_validate self.auto_extract_entities = auto_extract_entities - self.forceDcm2niix = forceDcm2niix + self.force_dcm2niix = force_dcm2niix self.logLevel = log_level # logging setup @@ -113,7 +113,7 @@ def run(self): check_latest() check_latest("dcm2niix") - dcm2niix.run(self.forceDcm2niix) + dcm2niix.run(self.force_dcm2niix) sidecars = [] for filename in dcm2niix.sidecarFiles: diff --git a/dcm2bids/dcm2niix_gen.py b/dcm2bids/dcm2niix_gen.py index 0707bc60..15ab1917 100644 --- a/dcm2bids/dcm2niix_gen.py +++ b/dcm2bids/dcm2niix_gen.py @@ -25,7 +25,7 @@ class Dcm2niixGen(object): """ def __init__( - self, dicomDirs, bidsDir, participant=None, options=DEFAULT.dcm2niixOptions + self, dicomDirs, bidsDir, participant=None, options=DEFAULT.dcm2niixOptions, helper = False ): self.logger = logging.getLogger(__name__) @@ -35,7 +35,7 @@ def __init__( self.bidsDir = bidsDir self.participant = participant self.options = options - + self.helper = helper @property def outputDir(self): """ @@ -43,10 +43,12 @@ def outputDir(self): A directory to save all the output files of dcm2niix """ tmpDir = self.participant.prefix if self.participant else DEFAULT.helperDir + tmpDir = self.bidsDir / DEFAULT.tmpDirName / tmpDir + if self.helper: + tmpDir = self.bidsDir + return tmpDir - return self.bidsDir / DEFAULT.tmpDirName / tmpDir - - def run(self, force=False): + def run(self, force = False, helper = False): """ Run dcm2niix if necessary Args: @@ -69,8 +71,6 @@ def run(self, force=False): shutil.rmtree(self.outputDir, ignore_errors=True) - # os.makedirs(self.outputDir, exist_ok=True) - # python2 compatibility if not os.path.exists(self.outputDir): os.makedirs(self.outputDir) @@ -79,11 +79,9 @@ def run(self, force=False): elif oldOutput: self.logger.warning("Previous dcm2niix directory output found:") self.logger.warning(self.outputDir) - self.logger.warning("Use --forceDcm2niix to rerun dcm2niix") + self.logger.warning("Use --force_dcm2niix to rerun dcm2niix \n") else: - # os.makedirs(self.outputDir, exist_ok=True) - # python2 compatibility if not os.path.exists(self.outputDir): os.makedirs(self.outputDir) diff --git a/dcm2bids/utils/args.py b/dcm2bids/utils/args.py index 153648a5..5b7db370 100644 --- a/dcm2bids/utils/args.py +++ b/dcm2bids/utils/args.py @@ -1,7 +1,8 @@ # -*- coding: utf-8 -*- -import os import shutil +from pathlib import Path +import os def assert_dirs_empty(parser, args, required): @@ -18,34 +19,26 @@ def assert_dirs_empty(parser, args, required): required: string or list of paths to files Required paths to be checked. """ - def check(path): - if os.path.isdir(path): - if os.listdir(path): + def check(path: Path): + if path.is_dir(): + if any(path.iterdir()): if not args.overwrite: parser.error( - f"Output directory {path} isn't empty, so some files " - "could be overwritten or deleted.\nRerun the command" - " with --force option to overwrite " - "existing output files.") + f"Output directory {path}{os.sep} isn't empty, so some files " + "could be overwritten or deleted.\nRerun the command " + "with --force option to overwrite " + "existing output files.") else: - for the_file in os.listdir(path): - file_path = os.path.join(path, the_file) - try: - if os.path.isfile(file_path): - os.unlink(file_path) - elif os.path.isdir(file_path): - shutil.rmtree(file_path) - except Exception as e: - print(e) + shutil.rmtree(path) if isinstance(required, str): - required = [required] + required = Path(required) - for cur_dir in required: - check(cur_dir) + for cur_dir in [required]: + check(cur_dir) def add_overwrite_arg(parser): parser.add_argument( '--force', dest='overwrite', action='store_true', - help='Force overwriting of the output files.') + help='Force overwriting of the output files.') \ No newline at end of file diff --git a/dcm2bids/utils/utils.py b/dcm2bids/utils/utils.py index 815ef8b4..58003119 100644 --- a/dcm2bids/utils/utils.py +++ b/dcm2bids/utils/utils.py @@ -27,7 +27,7 @@ class DEFAULT(object): bids_validate = False auto_extract_entities = False clobber = False - forceDcm2niix = False + force_dcm2niix = False defaceTpl = None logLevel = "WARNING" From a6e8e33f1a5086ad1eeeeac827b370603ac30ecd Mon Sep 17 00:00:00 2001 From: Samuel Guay Date: Fri, 23 Jun 2023 05:47:20 -0400 Subject: [PATCH 2/2] pep8ed --- dcm2bids/cli/dcm2bids_helper.py | 35 ++++++++++++++++++++++----------- dcm2bids/dcm2niix_gen.py | 16 +++++++++------ dcm2bids/utils/args.py | 10 +++++----- 3 files changed, 38 insertions(+), 23 deletions(-) diff --git a/dcm2bids/cli/dcm2bids_helper.py b/dcm2bids/cli/dcm2bids_helper.py index 3c09681e..c7c78a6d 100644 --- a/dcm2bids/cli/dcm2bids_helper.py +++ b/dcm2bids/cli/dcm2bids_helper.py @@ -8,14 +8,14 @@ import argparse import sys import os -from os.path import join as opj from pathlib import Path from datetime import datetime from dcm2bids.dcm2niix_gen import Dcm2niixGen -from dcm2bids.utils.args import add_overwrite_arg, assert_dirs_empty +from dcm2bids.utils.args import assert_dirs_empty from dcm2bids.utils.utils import DEFAULT from dcm2bids.utils.logger import setup_logging + def _build_arg_parser(): p = argparse.ArgumentParser(description=__doc__, epilog=DEFAULT.doc, formatter_class=argparse.RawTextHelpFormatter) @@ -26,15 +26,17 @@ def _build_arg_parser(): p.add_argument("-o", "--output_dir", required=False, - default=Path(DEFAULT.cliOutputDir) / DEFAULT.tmpDirName / DEFAULT.helperDir, - help=f'Output directory. (Default: [%(default)s]') + default=Path(DEFAULT.cliOutputDir) / DEFAULT.tmpDirName / + DEFAULT.helperDir, + help="Output directory. (Default: [%(default)s]") p.add_argument("-n", "--nest", nargs="?", const=True, default=False, required=False, - help=f"Nest a directory in . Useful if many helper runs are needed\n" - f"to make a config file due to slight variations in MRI acquisitions.\n" - f"Defaults to DICOM_DIR if no name is provided.\n" - f"(Default: [%(default)s])") + help="Nest a directory in . Useful if many helper " + "runs are needed\nto make a config file due to slight " + "variations in MRI acquisitions.\n" + "Defaults to DICOM_DIR if no name is provided.\n" + "(Default: [%(default)s])") p.add_argument('--force', '--force_dcm2niix', dest='overwrite', action='store_true', @@ -42,18 +44,26 @@ def _build_arg_parser(): return p + def main(): """Let's go""" parser = _build_arg_parser() args = parser.parse_args() out_dir = Path(args.output_dir) - log_path = Path(DEFAULT.cliOutputDir) / DEFAULT.tmpDirName / "log" / f"helper_{datetime.now().isoformat().replace(':', '')}.log" + log_path = (Path(DEFAULT.cliOutputDir) + / DEFAULT.tmpDirName + / "log" + / f"helper_{datetime.now().isoformat().replace(':', '')}.log") if args.nest: if isinstance(args.nest, str): - log_path = Path(str(log_path).replace("helper_", f"helper_{args.nest.replace(os.path.sep, '-')}_")) + log_path = Path( + str(log_path).replace("helper_", + f"helper_{args.nest.replace(os.path.sep, '-')}_")) out_dir = out_dir / args.nest else: - log_path = Path(str(log_path).replace("helper_", f"helper_{args.dicom_dir[0].replace(os.path.sep, '-')}_")) + log_path = Path(str(log_path).replace( + "helper_", f"helper_{args.dicom_dir[0].replace(os.path.sep, '-')}_") + ) out_dir = out_dir / args.dicom_dir[0] log_path.parent.mkdir(parents=True, exist_ok=True) @@ -64,7 +74,7 @@ def main(): assert_dirs_empty(parser, args, out_dir) - app = Dcm2niixGen(dicomDirs = args.dicom_dir, bidsDir = out_dir, helper = True) + app = Dcm2niixGen(dicomDirs=args.dicom_dir, bidsDir=out_dir, helper=True) rsl = app.run(force=args.overwrite) logger.info(f"Helper files in: {out_dir}\n") @@ -72,5 +82,6 @@ def main(): return rsl + if __name__ == "__main__": main() diff --git a/dcm2bids/dcm2niix_gen.py b/dcm2bids/dcm2niix_gen.py index 15ab1917..e82b2398 100644 --- a/dcm2bids/dcm2niix_gen.py +++ b/dcm2bids/dcm2niix_gen.py @@ -25,17 +25,21 @@ class Dcm2niixGen(object): """ def __init__( - self, dicomDirs, bidsDir, participant=None, options=DEFAULT.dcm2niixOptions, helper = False + self, + dicomDirs, + bidsDir, + participant=None, + options=DEFAULT.dcm2niixOptions, + helper=False ): self.logger = logging.getLogger(__name__) - self.sidecarsFiles = [] - self.dicomDirs = dicomDirs self.bidsDir = bidsDir self.participant = participant self.options = options self.helper = helper + @property def outputDir(self): """ @@ -48,7 +52,7 @@ def outputDir(self): tmpDir = self.bidsDir return tmpDir - def run(self, force = False, helper = False): + def run(self, force=False, helper=False): """ Run dcm2niix if necessary Args: @@ -60,7 +64,7 @@ def run(self, force = False, helper = False): """ try: oldOutput = os.listdir(self.outputDir) != [] - except: + except Exception: oldOutput = False if oldOutput and force: @@ -99,7 +103,7 @@ def execute(self): try: output = output.decode() - except: + except Exception: pass self.logger.debug("\n%s", output) diff --git a/dcm2bids/utils/args.py b/dcm2bids/utils/args.py index 5b7db370..55a93c30 100644 --- a/dcm2bids/utils/args.py +++ b/dcm2bids/utils/args.py @@ -25,20 +25,20 @@ def check(path: Path): if not args.overwrite: parser.error( f"Output directory {path}{os.sep} isn't empty, so some files " - "could be overwritten or deleted.\nRerun the command " - "with --force option to overwrite " - "existing output files.") + "could be overwritten or deleted.\nRerun the command " + "with --force option to overwrite " + "existing output files.") else: shutil.rmtree(path) if isinstance(required, str): required = Path(required) - for cur_dir in [required]: check(cur_dir) + def add_overwrite_arg(parser): parser.add_argument( '--force', dest='overwrite', action='store_true', - help='Force overwriting of the output files.') \ No newline at end of file + help='Force overwriting of the output files.')