Repository: DIAGNijmegen/picai_prep (language: python)

Issue:
Move stuff around
- Collect errors in single file
- Put important functions on top, then logging functions, then builtins like __repr__


Discussion / hints:

Fix patch:
diff --git a/src/picai_prep/archive.py b/src/picai_prep/archive.py
deleted file mode 100644
index 7686821..0000000
--- a/src/picai_prep/archive.py
+++ /dev/null
@@ -1,128 +0,0 @@
-#  Copyright 2022 Diagnostic Image Analysis Group, Radboudumc, Nijmegen, The Netherlands
-#
-#  Licensed under the Apache License, Version 2.0 (the "License");
-#  you may not use this file except in compliance with the License.
-#  You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-#  Unless required by applicable law or agreed to in writing, software
-#  distributed under the License is distributed on an "AS IS" BASIS,
-#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-#  See the License for the specific language governing permissions and
-#  limitations under the License.
-
-
-import datetime
-import logging
-import os
-from abc import ABC, abstractmethod
-from pathlib import Path
-from typing import Dict, Optional
-
-from picai_prep.data_utils import PathLike
-from picai_prep.utilities import plural
-
-
-class ArchiveConverter(ABC):
-    def __init__(
-        self,
-        input_path: PathLike,
-        output_path: PathLike,
-        num_threads: int = 4,
-        silent=False
-    ):
-        super().__init__()
-        self.items = []
-        self._history = None
-
-        self.input_dir = Path(os.path.abspath(input_path))
-        self.output_dir = Path(os.path.abspath(output_path))
-        self.output_dir.mkdir(exist_ok=True, parents=True)
-        self.num_threads = num_threads
-        self.silent = silent
-        self._start_time = datetime.datetime.now()
-
-        logfile = self.output_dir / f'picai_prep_{datetime.datetime.now().strftime("%Y%m%d%H%M%S")}.log'
-        logging.basicConfig(filemode='w', level=logging.INFO, format='%(message)s', filename=logfile)
-
-        self.info(f'Program started at {self._start_time.isoformat()}', force=True)
-        self.info(f"Output directory set to {self.output_dir.absolute().as_posix()}, writing log to {logfile}")
-
-    @abstractmethod
-    def convert(self):
-        raise NotImplementedError()
-
-    @property
-    def valid_items(self) -> filter:
-        return filter(lambda a: a.get('error', None) is None, self.items)
-
-    @property
-    def has_valid_items(self):
-        return self.num_valid_items > 0
-
-    @property
-    def num_valid_items(self):
-        return len(list(self.valid_items))
-
-    def valid_items_str(self, syntax: str = 'item') -> str:
-        return plural(self._history.num_items, syntax)
-
-    def item_log(self, item, msg: str):
-        self._history.add(msg)
-        if item:
-            ie = self.item_log_value(item)
-            if ie:
-                logging.error('\n\t' + ie)
-
-    @abstractmethod
-    def item_log_value(self, item: Dict[str, str]):
-        raise NotImplementedError()
-
-    def info(self, *msg, force=False):
-        msg = ' '.join([str(m) for m in msg if m])
-        logging.info('\n' + msg)
-        if not self.silent or force:
-            print(msg, '\n')
-
-    def complete(self):
-        end_time = datetime.datetime.now()
-        program_duration = end_time - self._start_time
-        self.info(f'Program ended at {end_time.isoformat()} (took {program_duration})', force=True)
-
-    def next_history(self):
-        self._history = History(len(list(self.items))) if self._history is None else self._history.next()
-
-    def get_history_report(self):
-        return self._history.report()
-
-
-class History:
-    def __init__(self, num_items: int, past: Optional["History"] = None):
-        """
-        Parameters:
-        - num_items: number of items remaining in the preprocessing pipeline
-        - past: History object of previous step in the preprocessing pipeline
-
-        The ledger holds the number of errors/messages of a specific type,
-        which allows to show aggregates to the user.
-        """
-        self.num_items = num_items
-        self.ledger = dict()
-        self.past = past
-
-    def report(self):
-        report = []
-        if self.past is not None:
-            prev, now = self.past.num_items, self.num_items
-            report += [f"{prev - now} ignored. ({prev} -> {now})"] if prev - now > 0 else [' ']
-
-        report += [f"\t{item_error}: {count}" for item_error, count in self.ledger.items()]
-        return '\n'.join(report)
-
-    def add(self, key: str):
-        self.ledger[key] = self.ledger.get(key, 0) + 1
-        self.num_items -= 1
-
-    def next(self):
-        return History(self.num_items, self)
diff --git a/src/picai_prep/converter.py b/src/picai_prep/converter.py
new file mode 100644
index 0000000..da62767
--- /dev/null
+++ b/src/picai_prep/converter.py
@@ -0,0 +1,86 @@
+import logging
+from abc import ABC, abstractmethod
+from concurrent.futures import ThreadPoolExecutor, as_completed
+from dataclasses import dataclass, field
+from datetime import datetime
+from pathlib import Path
+from typing import Any, Dict, List, Optional
+
+from tqdm import tqdm
+
+
+@dataclass
+class Case(ABC):
+    patient_id: str
+    study_id: str
+
+    error: Optional[Exception] = None
+
+    _log: List[str] = field(default_factory=list)
+
+    @abstractmethod
+    def convert_item(self, **kargs):
+        """"
+        Execute conversion process, please implement this.
+        """
+        raise NotImplementedError()
+
+    def convert(self, **kargs):
+        """"
+        Execute conversion process, while handling errors.
+        Please override the convert_item method to implement the conversion process.
+        """
+        try:
+            self.convert_item(**kargs)
+        except Exception as e:
+            self.invalidate(e)
+        finally:
+            return self.compile_log()
+
+    @property
+    def subject_id(self):
+        return f'{self.patient_id}_{self.study_id}'
+
+    @property
+    def is_valid(self):
+        return self.error is None
+
+    def invalidate(self, error: Exception):
+        self.error = error
+
+    def write_log(self, msg: str):
+        self._log.append(msg)
+
+    @abstractmethod
+    def compile_log(self):
+        raise NotImplementedError()
+
+    def __repr__(self):
+        return f'Case({self.patient_id}_{self.study_id})'
+
+
+class Converter:
+    @staticmethod
+    def initialize_log(output_dir: Path, verbose: int):
+        if verbose >= 1:
+            logfile = output_dir / f'picai_prep_{datetime.now().strftime("%Y%m%d%H%M%S")}.log'
+            logging.basicConfig(level=logging.INFO, format='%(message)s', filename=logfile)
+            logging.info(f'Output directory set to {output_dir.absolute().as_posix()}')
+            print(f'Writing log to {logfile.absolute()}')
+        else:
+            logging.disable(logging.INFO)
+
+    @staticmethod
+    def _convert(title: str, cases: List[Case], parameters: Dict[str, Any], num_threads: int = 4):
+        start_time = datetime.now()
+        logging.info(f'{title} conversion started at {start_time.isoformat()}\n')
+
+        with ThreadPoolExecutor(max_workers=num_threads) as pool:
+            futures = {pool.submit(case.convert, **parameters): case for case in cases}
+            for future in tqdm(as_completed(futures), total=len(cases)):
+                case_log = future.result()
+                if case_log:
+                    logging.info(case_log)
+
+        end_time = datetime.now()
+        logging.info(f'{title} conversion ended at {end_time.isoformat()}\n\t(runtime {end_time - start_time})')
diff --git a/src/picai_prep/dcm2mha.py b/src/picai_prep/dcm2mha.py
index c7a5d09..62c8736 100644
--- a/src/picai_prep/dcm2mha.py
+++ b/src/picai_prep/dcm2mha.py
@@ -15,9 +15,7 @@
 import logging
 import os
 import re
-from concurrent.futures import ThreadPoolExecutor, as_completed
 from dataclasses import dataclass, field
-from datetime import datetime
 from pathlib import Path
 from typing import Callable, Dict, List, Optional, Sequence, Union
 
@@ -25,64 +23,31 @@
 import numpy as np
 import pydicom.errors
 import SimpleITK as sitk
-from tqdm import tqdm
 
+from picai_prep.converter import Case, Converter
 from picai_prep.data_utils import PathLike, atomic_image_write
+from picai_prep.errors import (ArchiveItemPathNotFoundError,
+                               CriticalErrorInSiblingError,
+                               MissingDICOMFilesError, NoMappingsApplyError,
+                               UnreadableDICOMError)
 from picai_prep.utilities import (dcm2mha_schema, dicom_tags,
                                   get_pydicom_value, lower_strip,
-                                  make_sitk_readers, metadata_defaults, plural)
+                                  make_sitk_readers, plural)
 
 Metadata = Dict[str, str]
 Mapping = Dict[str, List[str]]
 Mappings = Dict[str, Mapping]
 
 
-class SeriesException(Exception):
-    """Base Exception for errors in an item (series within a case)"""
-
-    def __init__(self, message: str):
-        super().__init__(message)
-
-    def __str__(self):
-        return f'{type(self).__name__}: {", ".join([a for a in self.args])}'
-
-
-class MissingDICOMFilesError(SeriesException):
-    """Exception raised when a DICOM series has missing DICOM slices"""
-
-    def __init__(self, path: PathLike):
-        super().__init__(f"Missing DICOM slices detected in {path}")
-
-
-class NoMappingsApplyError(SeriesException):
-    """Exception raised when no mappings apply to the case"""
-
-    def __init__(self):
-        super().__init__('None of the provided mappings apply to this item')
-
-
-class UnreadableDICOMError(SeriesException):
-    """Exception raised when a DICOM series could not be loaded"""
-
-    def __init__(self, path: PathLike):
-        super().__init__(f'Could not read {path} using either SimpleITK or pydicom')
-
-
-class ArchiveItemPathNotFoundError(SeriesException):
-    """Exception raised when a DICOM series could not be found"""
-
-    def __init__(self, path: PathLike):
-        super().__init__(f"Provided archive item path not found ({path})")
-
-
 @dataclass
 class Dicom2MHASettings:
     mappings: Dict[str, Dict[str, List[str]]]
-    num_threads: int = 4
     verify_dicom_filenames: bool = True
     allow_duplicates: bool = False
     metadata_match_func: Optional[Callable[[Metadata, Mappings], bool]] = None
     values_match_func: Union[str, Callable[[str, str], bool]] = "lower_strip_equals"
+    num_threads: int = 4
+    verbose: int = 1
 
     def __post_init__(self):
         # Validate the mappings
@@ -108,10 +73,8 @@ class Series:
     mappings: List[str] = field(default_factory=list)
 
     error: Optional[Exception] = None
-    _log: List[str] = field(default_factory=list)
 
-    def __repr__(self):
-        return f"Series({self.path.name})"
+    _log: List[str] = field(default_factory=list)
 
     def __post_init__(self):
         if not self.path.exists():
@@ -119,13 +82,6 @@ def __post_init__(self):
         if not self.path.is_dir():
             raise NotADirectoryError(self.path)
 
-    @property
-    def is_valid(self) -> bool:
-        return self.error is None
-
-    def write_log(self, msg: str):
-        self._log.append(msg)
-
     def verify_dicom_filenames(self, filenames: List[PathLike]) -> bool:
         """Verify DICOM filenames have increasing numbers, with no gaps"""
         vdcms = [d.rsplit('.', 1)[0] for d in filenames]
@@ -240,73 +196,49 @@ def values_match_func(needle: str, haystack: str) -> bool:
             raise NoMappingsApplyError()
         self.write_log(f'Applied mappings [{", ".join(self.mappings)}]')
 
+    def write_log(self, msg: str):
+        self._log.append(msg)
 
-class Case:
-    pass
+    def compile_log(self):
+        log = [f'\t{item}' for item in self._log]
+        return '\n'.join([self.path.as_posix()] + log + [f'\tFATAL: {self.error}\n' if not self.is_valid else ''])
+
+    @property
+    def is_valid(self):
+        return self.error is None
+
+    def __repr__(self):
+        return f"Series({self.path.name})"
 
 
 @dataclass
-class Dicom2MHACase(Case):
+class _Dicom2MHACaseBase:
     input_dir: Path
-    patient_id: str
-    study_id: str
     paths: List[PathLike]
     settings: Dicom2MHASettings
 
-    def __repr__(self):
-        return f'Case({self.subject_id})'
 
-    @property
-    def valid_series(self):
-        return [item for item in self.series if item.is_valid]
-
-    @property
-    def subject_id(self) -> str:
-        return f"{self.patient_id}_{self.study_id}"
-
-    def invalidate(self):
-        for serie in self.valid_series:
-            serie.error = SeriesException('Invalidated due to critical error in sibling')
-
-    def write_log(self, msg: str):
-        self._log.append(msg)
-
-    def compile_log(self):
-        divider = '=' * 120
-        log = [divider,
-               f'CASE {self.subject_id}',
-               f'\tPATIENT ID\t{self.patient_id}',
-               f'\tSTUDY ID\t{self.study_id}\n']
-        log += self._log
-        log += ['\nSERIES', divider.replace('=', '-')]
-        for i, serie in enumerate(self.series):
-            log.append(f'({i}) {serie.path.as_posix()}')
-            log.extend([f'\t{item}' for item in serie._log])
-            log.append(f'\tFATAL: {serie.error}\n' if not serie.is_valid else '')
-        return '\n'.join(log)
+@dataclass
+class Dicom2MHACase(Case, _Dicom2MHACaseBase):
+    series: List[Series] = field(default_factory=list)
 
-    def convert(self, output_dir):
-        try:
-            self.initialize()
-            self.extract_metadata()
-            self.apply_mappings()
-            self.resolve_duplicates()
-            self.process_and_write(output_dir)
-        except Exception as e:
-            self.invalidate()
-            logging.error(str(e))
-        finally:
-            return self.compile_log()
+    def convert_item(self, output_dir: Path) -> None:
+        self.initialize()
+        self.extract_metadata()
+        self.apply_mappings()
+        self.resolve_duplicates()
+        self.process_and_write(output_dir)
 
     def initialize(self):
-        self.series: List[Series] = []
-        self._log = [f'Importing {plural(len(self.paths), "serie")}']
+        self.write_log(f'Importing {plural(len(self.paths), "serie")}')
 
         full_paths = set()
         for path in self.paths:
             full_path = self.input_dir / path
             serie = Series(full_path, self.patient_id, self.study_id)
             try:
+                # if we find duplicate paths with the same patient and study id,
+                # invalidate this series and continue for logging purposes
                 if path in full_paths:
                     raise FileExistsError(path)
                 full_paths.add(full_path)
@@ -317,8 +249,8 @@ def initialize(self):
                 self.write_log(f'\t+ ({len(self.series)}) {full_path}')
                 self.series.append(serie)
 
-        if not all([serie.is_valid for serie in self.series]):
-            self.invalidate()
+        if not self.is_valid:
+            self.invalidate(CriticalErrorInSiblingError)
 
     def extract_metadata(self):
         self.write_log(f'Extracting metadata from {plural(len(self.valid_series), "serie")}')
@@ -405,7 +337,8 @@ def process_and_write(self, output_dir: Path):
                 try:
                     image = read_image_series(serie.path)
                 except Exception as e:
-                    serie.write_log(f'Skipped "{mapping}", reading DICOM sequence failed, maybe corrupt data? Error: {e}')
+                    serie.write_log(
+                        f'Skipped "{mapping}", reading DICOM sequence failed, maybe corrupt data? Error: {e}')
                     logging.error(str(e))
                     errors.append(i)
                 else:
@@ -425,8 +358,61 @@ def process_and_write(self, output_dir: Path):
                        f'\t({plural(len(errors), "error")}{f" {errors}" if len(errors) > 0 else ""}, '
                        f'{len(skips)} skipped{f" {skips}" if len(skips) > 0 else ""})')
 
+    def invalidate(self):
+        for serie in self.valid_series:
+            serie.error = CriticalErrorInSiblingError()
+
+    @property
+    def subject_id(self) -> str:
+        return f"{self.patient_id}_{self.study_id}"
+
+    @property
+    def is_valid(self):
+        return all([serie.is_valid for serie in self.series])
 
-class Dicom2MHAConverter:
+    @property
+    def valid_series(self):
+        return [item for item in self.series if item.is_valid]
+
+    def write_log(self, msg: str):
+        self._log.append(msg)
+
+    def compile_log(self):
+        """For questions: Stan.Noordman@Radboudumc.nl"""
+        if self.settings.verbose == 0:
+            return
+
+        divider = '=' * 120
+        summary = {}
+        serie_log = []
+
+        # summarize each serie's log (if any)
+        for i, serie in enumerate(self.series):
+            serie_log.append(f'({i}) {serie.compile_log()}')
+            if serie.error:
+                summary[serie.error.__class__.__name__] = summary.get(serie.error.__class__.__name__, []) + [i]
+
+        # these are the errors that are not fatal
+        ignored_errors = {e.__name__ for e in [NoMappingsApplyError]}
+
+        # check if we should log any errors
+        if len(set(summary.keys()).difference(ignored_errors)) > 0 or self.settings.verbose >= 2:
+            # don't worry, this just looks nice in the log
+            return '\n'.join([divider,
+                              f'CASE {self.patient_id}_{self.study_id}',
+                              f'\tPATIENT ID\t{self.patient_id}',
+                              f'\tSTUDY ID\t{self.study_id}\n',
+                              *self._log,
+                              '\nSERIES', divider.replace('=', '-'),
+                              'Errors found:',
+                              *[f'\t{key}: {value}' for key, value in summary.items()],
+                              '', *serie_log, ''])
+
+    def __repr__(self):
+        return f'Case({self.subject_id})'
+
+
+class Dicom2MHAConverter(Converter):
     def __init__(
         self,
         input_dir: PathLike,
@@ -436,28 +422,34 @@ def __init__(
         """
         Parameters
         ----------
-        - input_dir: path to the DICOM archive. Used as base path for the relative paths
-            of the archive items.
-        - output_dir: path to store the MHA archive.
-        - dcm2mha_settings: object with mappings, cases and optional parameters. May be
-            a dictionary containing `mappings`, `archive`, and optionally `options`,
-            or a path to a JSON file with these elements.
+        input_dir: PathLike
+            path to the DICOM archive. Used as base path for the relative paths of the archive items.
+        output_dir: PathLike
+            path to store the resulting MHA archive.
+        dcm2mha_settings: Union[PathLike, Dict], default: None
+            object with mappings, cases and optional parameters. May be a dictionary containing mappings, archive,
+            and optionally options, or a path to a JSON file with these elements.
             - mappings: criteria to map DICOM sequences to their MHA counterparts
-            - cases: list of DICOM sequences in the DICOM archive. Each case should contain:
-                - patient_id: unique patient identifier
-                - study_id: unique study identifier
-                - path: path to DICOM sequence.
+            - cases: list of DICOM sequences in the DICOM archive. Each case is to be an object with a patient_id,
+                study_id and path to DICOM sequence
             - options: (optional)
-                - num_threads: number of multithreading threads. Default: 4.
-                - verify_dicom_filenames: whether to check if DICOM filenames contain consequtive
-                    numbers. Default: True
+                - num_threads: number of multithreading threads.
+                    Default: 4.
+                - verify_dicom_filenames: whether to check if DICOM filenames contain consecutive
+                    numbers.
+                    Default: True
                 - allow_duplicates: whether multiple DICOM series can map to the same MHA postfix.
                     Default: False
                 - metadata_match_func: method to match DICOM metadata to MHA sequences. Only use
                     this if you know what you're doing.
-                - values_match_func: criterium to consider two values a match, when comparing the
+                    Default: None
+                - values_match_func: criteria to consider two values a match, when comparing the
                     value from the DICOM metadata against the provided allowed vaues in the mapping.
-
+                    Default: None
+                - verbose: control logfile verbosity. 0 does not output a logfile,
+                    1 logs cases which have critically failed, 2 logs all cases (may lead to
+                    very large log files)
+                    Default: 1
         """
         self.input_dir = Path(input_dir)
         self.output_dir = Path(output_dir)
@@ -476,34 +468,28 @@ def __init__(
 
         self.cases = self._init_cases(dcm2mha_settings['archive'])
 
-        logfile = self.output_dir / f'picai_prep_{datetime.now().strftime("%Y%m%d%H%M%S")}.log'
-        logging.basicConfig(level=logging.INFO, format='%(message)s', filename=logfile)
-        logging.info(f'Output directory set to {self.output_dir.absolute().as_posix()}')
-        print(f'Writing log to {logfile.absolute()}')
+        self.initialize_log(self.output_dir, self.settings.verbose)
 
     def _init_cases(self, archive: List[Dict]) -> List[Dicom2MHACase]:
         cases = {}
         for item in archive:
-            key = tuple(item[id] for id in metadata_defaults.keys())  # (patient_id, study_id)
+            key = tuple(item[id] for id in ["patient_id", "study_id"])
             cases[key] = cases.get(key, []) + [item['path']]
-
         return [
-            Dicom2MHACase(self.input_dir, patient_id, study_id, paths, self.settings)
+            Dicom2MHACase(input_dir=self.input_dir, patient_id=patient_id,
+                          study_id=study_id, paths=paths, settings=self.settings)
             for (patient_id, study_id), paths in cases.items()
         ]
 
     def convert(self):
-        start_time = datetime.now()
-        logging.info(f'Program started at {start_time.isoformat()}\n')
-
-        with ThreadPoolExecutor(max_workers=self.settings.num_threads) as pool:
-            futures = {pool.submit(case.convert, self.output_dir): case for case in self.cases}
-            for future in tqdm(as_completed(futures), total=len(self.cases)):
-                case_log = future.result()
-                logging.info(case_log)
-
-        end_time = datetime.now()
-        logging.info(f'Program ended at {end_time.isoformat()}\n\t(runtime {end_time - start_time})')
+        self._convert(
+            title='Dicom2MHA',
+            cases=self.cases,
+            parameters={
+                'output_dir': self.output_dir
+            },
+            num_threads=self.settings.num_threads,
+        )
 
 
 def read_image_series(image_series_path: PathLike) -> sitk.Image:
diff --git a/src/picai_prep/errors.py b/src/picai_prep/errors.py
new file mode 100644
index 0000000..fe90012
--- /dev/null
+++ b/src/picai_prep/errors.py
@@ -0,0 +1,46 @@
+from picai_prep.data_utils import PathLike
+
+
+class ConverterException(Exception):
+    """Base Exception for errors in an item (series within a case)"""
+
+    def __init__(self, message: str):
+        super().__init__(message)
+
+    def __str__(self):
+        return f'{type(self).__name__}: {", ".join([a for a in self.args])}'
+
+
+class ArchiveItemPathNotFoundError(ConverterException):
+    """Exception raised when a archive path could not be found"""
+
+    def __init__(self, path: PathLike):
+        super().__init__(f"Provided archive item path not found ({path})")
+
+
+class CriticalErrorInSiblingError(ConverterException):
+    """Exception raised when a critical error in a sibling item occurs"""
+
+    def __init__(self):
+        super().__init__("Critical error in sibling item")
+
+
+class MissingDICOMFilesError(ConverterException):
+    """Exception raised when a DICOM series has missing DICOM slices"""
+
+    def __init__(self, path: PathLike):
+        super().__init__(f"Missing DICOM slices detected in {path}")
+
+
+class NoMappingsApplyError(ConverterException):
+    """Exception raised when no mappings apply to the case"""
+
+    def __init__(self):
+        super().__init__('None of the provided mappings apply to this item')
+
+
+class UnreadableDICOMError(ConverterException):
+    """Exception raised when a DICOM series could not be loaded"""
+
+    def __init__(self, path: PathLike):
+        super().__init__(f'Could not read {path} using either SimpleITK or pydicom')
diff --git a/src/picai_prep/mha2nnunet.py b/src/picai_prep/mha2nnunet.py
index b453735..dac30ee 100755
--- a/src/picai_prep/mha2nnunet.py
+++ b/src/picai_prep/mha2nnunet.py
@@ -13,329 +13,255 @@
 #  limitations under the License.
 
 
-import dataclasses
 import json
-import os
-from concurrent.futures import ThreadPoolExecutor, as_completed
-from dataclasses import dataclass
+import logging
+from dataclasses import dataclass, field
 from pathlib import Path
-from typing import Any, Callable, Dict, List, Optional, Tuple
+from typing import Any, Callable, Dict, List, Optional, Union
 
 import jsonschema
 import SimpleITK as sitk
-from tqdm import tqdm
 
-from picai_prep.archive import ArchiveConverter
+from picai_prep.converter import Case, Converter
 from picai_prep.data_utils import PathLike, atomic_image_write
 from picai_prep.preprocessing import PreprocessingSettings, Sample
 from picai_prep.utilities import mha2nnunet_schema, plural
 
 
 @dataclass
-class ConversionItem():
-    """Class to hold conversion information"""
-    input_dir: PathLike
-    annotations_dir: Optional[PathLike]
-    patient_id: str
-    study_id: str
-    scan_paths: List[PathLike]
-    out_dir_scans: PathLike = "imagesTr"
-    out_dir_annot: PathLike = "labelsTr"
-    annotation_path: Optional[PathLike] = None
-    all_scan_properties: List[Dict[str, Any]] = dataclasses.field(default_factory=list)
-    label_properties: Dict[str, Any] = dataclasses.field(default_factory=dict)
-    error: Optional[Tuple[str, str]] = None
-    skip: bool = False
+class MHA2nnUNetSettings:
+    dataset_json: Dict[str, Any]
+    preprocessing: PreprocessingSettings
+    scans_dirname: PathLike = "imagesTr"
+    annotation_dirname: PathLike = "labelsTr"
+    annotation_preprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None
+    annotation_postprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None
+    scan_preprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None
+    scan_postprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None
+    num_threads: int = 4
+    verbose: int = 1
+
+    @property
+    def task_name(self) -> str:
+        return self.dataset_json['task']
+
+
+@dataclass
+class _MHA2nnUNetCaseBase:
+    scans_dir: Path
+    annotations_dir: Path
+    scan_paths: List[Path]
+    settings: MHA2nnUNetSettings
+
+
+@dataclass
+class MHA2nnUNetCase(Case, _MHA2nnUNetCaseBase):
+    annotation_path: Optional[Path] = None
+    verified_scan_paths: List[Path] = field(default_factory=list)
 
     def __post_init__(self):
-        # convert input paths
-        self.input_dir = Path(self.input_dir)
-        if self.annotations_dir is not None:
-            self.annotations_dir = Path(self.annotations_dir)
-        self.scan_paths = [Path(path) for path in self.scan_paths]
-        self.out_dir_scans = Path(self.out_dir_scans)
-        self.out_dir_annot = Path(self.out_dir_annot)
-
-        output_paths = []
-        # check if all scans exist and append to conversion plan
-        for i, scan_path in enumerate(self.scan_paths):
-            # check (relative) path of input scans
-            path = self.input_dir / scan_path
-            if not os.path.exists(path):
-                self.error = (f"Scan not found at {path}", 'scan not found')
-                continue
+        if self.annotations_dir and self.annotation_path:
+            self.annotation_path = self.annotations_dir / self.annotation_path
 
-            output_path = self.out_dir_scans / f"{self.subject_id}_{i:04d}.nii.gz"
-            self.all_scan_properties += [{
-                'input_path': path,
-                'output_path': output_path,
-            }]
-            output_paths += [output_path]
+    def convert_item(self, scans_out_dir: Path, annotations_out_dir: Path) -> None:
+        self.initialize()
+        self.process_and_write(scans_out_dir, annotations_out_dir)
 
-        # check if annotation exists and append to conversion plan
-        if self.annotation_path is not None:
-            if self.annotations_dir is None:
-                self.annotations_dir = self.input_dir
+    def initialize(self):
+        self.write_log(f'Importing {plural(len(self.scan_paths), "scans")}')
 
+        missing_paths = []
+        for scan_path in self.scan_paths:
             # check (relative) path of input scans
-            path = self.annotations_dir / self.annotation_path
-            if not os.path.exists(path):
-                self.error = (f"Annotation not found at {path}", 'annotation not found')
-
-            output_path = self.out_dir_annot / f"{self.subject_id}.nii.gz"
-            self.label_properties = {
-                'input_path': path,
-                'output_path': output_path,
-            }
-            output_paths += [output_path]
+            path = self.scans_dir / scan_path
+            if not path.exists():
+                missing_paths.append(path)
+                continue
 
-        # check if all files already exist
-        if all([os.path.exists(path) for path in output_paths]):
-            self.skip = True
+            self.verified_scan_paths.append(path)
+            self.write_log(f'\t+ ({len(self.verified_scan_paths)}) {path}')
 
-    @property
-    def subject_id(self):
-        return f"{self.patient_id}_{self.study_id}"
+        if len(missing_paths) > 0:
+            raise FileNotFoundError(','.join([str(p) for p in missing_paths]))
 
+        if self.annotation_path:
+            self.write_log('Importing annotation')
+            if not self.annotation_path.exists():
+                raise FileNotFoundError(self.annotation_path)
 
-class MHA2nnUNetConverter(ArchiveConverter):
-    def __init__(
-        self,
-        input_path: PathLike,
-        output_path: PathLike,
-        settings_path: PathLike,
-        annotations_path: Optional[PathLike] = None,
-        lbl_preprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None,
-        lbl_postprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None,
-        scan_preprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None,
-        scan_postprocess_func: Optional[Callable[[sitk.Image], sitk.Image]] = None,
-        out_dir_scans: PathLike = "imagesTr",
-        out_dir_annot: PathLike = "labelsTr",
-        num_threads: int = 4,
-        silent: bool = False,
-    ):
-        """
-        Converts MHA archive into nnUNet raw data format, as specified by the conversion json.
-
-        Parameters:
-        - input_path: Root directory for input, e.g. /path/to/archive/
-        - output_path: Root directory for output
-        - archive: JSON mappings file
-                   Verifies whether archive.json returns a valid object prior to initialization
-        - silent: control verbosity of conversion process
-        """
-        super().__init__(
-            input_path=input_path,
-            output_path=output_path,
-            num_threads=num_threads,
-            silent=silent
-        )
-        if annotations_path is not None:
-            annotations_path = Path(annotations_path)
-        self.create_dataset_json = True
-
-        # read and verify conversion settings
-        with open(settings_path) as fp:
-            settings = json.load(fp)
-        jsonschema.validate(settings, mha2nnunet_schema, cls=jsonschema.Draft7Validator)
-
-        # store conversion settings
-        self.preprocessing_settings = PreprocessingSettings(**settings['preprocessing'])
-        self.items = settings['archive']
-        self.dataset_json = settings['dataset_json']
-        self.conversion_plan: List[ConversionItem] = []
-        self.out_dir_scans = out_dir_scans
-        self.out_dir_annot = out_dir_annot
-        self.annotations_path = annotations_path
-        self.lbl_preprocess_func = lbl_preprocess_func
-        self.lbl_postprocess_func = lbl_postprocess_func
-        self.scan_preprocess_func = scan_preprocess_func
-        self.scan_postprocess_func = scan_postprocess_func
-
-        self.next_history()  # create initial history step
-        self.info("Provided mha2nnunet archive is valid.", self.get_history_report())  # report number of items
-        self.next_history()  # next history
-
-    def _plan(self):
-        """
-        Build full conversion plan
-        """
-        # parse parameters
-        self.info(f"Starting preprocessing script for {self.task}.\n"
-                  + f"\tReading scans from {self.input_dir}\n"
-                  + (f"\tReading annotations from {self.annotations_path}\n" if self.annotations_path else "")
-                  + f"\nCreating preprocessing plan with {self.valid_items_str()}.")
-
-        total_scans, total_lbl = 0, 0
-
-        for item in self.items:
-            # parse conversion info and store result
-            conv_item = ConversionItem(
-                input_dir=self.input_dir,
-                annotations_dir=self.annotations_path,
-                out_dir_scans=self.output_dir / self.task / self.out_dir_scans,
-                out_dir_annot=self.output_dir / self.task / self.out_dir_annot,
-                **item
-            )
-            item['conversion_item'] = conv_item
-            if conv_item.error is not None:
-                message, error_type = conv_item.error
-                item['error'] = message
-                self.item_log(item, error_type)
-            else:
-                total_scans += len(item['scan_paths'])
-                total_lbl += 1 if 'annotation_path' in item else 0
-
-        self.info(f"Preprocessing plan completed, with {self.valid_items_str()} containing a total of "
-                  f"{plural(total_scans, 'scan')} and {plural(total_lbl, 'label')}.",
-                  self.get_history_report())
-
-    def _convert_item(self, item) -> bool:
-        conv_item = item['conversion_item']
-        if conv_item.skip:
-            # skip, already converted
-            return False
-
-        # read images
-        scans = []
-        for scan_properties in conv_item.all_scan_properties:
-            scans += [sitk.ReadImage(str(scan_properties['input_path']))]
-
-        # read label
-        lbl = None
-        if conv_item.label_properties:
-            lbl = sitk.ReadImage(str(conv_item.label_properties['input_path']))
+            self.write_log(f'\t+ {self.annotation_path}')
+
+    def process_and_write(self, scans_out_dir: Path, annotations_out_dir: Path):
+        self.write_log(f'Writing {plural(len(self.verified_scan_paths), "scan")}' + ' including annotation' if self.annotation_path else '')
+
+        scans = [sitk.ReadImage(path.as_posix()) for path in self.verified_scan_paths]
+        lbl = sitk.ReadImage(self.annotation_path.as_posix()) if self.annotation_path else None
 
         # set up Sample
         sample = Sample(
             scans=scans,
             lbl=lbl,
-            settings=self.preprocessing_settings,
-            lbl_preprocess_func=self.lbl_preprocess_func,
-            lbl_postprocess_func=self.lbl_postprocess_func,
-            scan_preprocess_func=self.scan_preprocess_func,
-            scan_postprocess_func=self.scan_postprocess_func,
-            name=conv_item.subject_id
+            settings=self.settings.preprocessing,
+            lbl_preprocess_func=self.settings.annotation_preprocess_func,
+            lbl_postprocess_func=self.settings.annotation_postprocess_func,
+            scan_preprocess_func=self.settings.scan_preprocess_func,
+            scan_postprocess_func=self.settings.scan_postprocess_func,
+            name=self.subject_id
         )
 
         # perform preprocessing
         sample.preprocess()
 
         # write images
-        for scan, scan_properties in zip(sample.scans, conv_item.all_scan_properties):
-            atomic_image_write(scan, path=scan_properties['output_path'], mkdir=True)
+        for i, scan in enumerate(sample.scans):
+            destination_path = scans_out_dir / f"{self.subject_id}_{i:04d}.nii.gz"
+            atomic_image_write(scan, path=destination_path, mkdir=True)
+            self.write_log(f'Wrote image to {destination_path}')
+
+        if lbl:
+            destination_path = annotations_out_dir / f"{self.subject_id}.nii.gz"
+            atomic_image_write(sample.lbl, path=annotations_out_dir / f"{self.subject_id}.nii.gz", mkdir=True)
+            self.write_log(f'Wrote annotation to {destination_path}')
+
+    def compile_log(self):
+        if self.settings.verbose == 0:
+            return None
 
-        if sample.lbl:
-            atomic_image_write(sample.lbl, path=conv_item.label_properties['output_path'], mkdir=True)
+        if self.is_valid or self.settings.verbose >= 2:
+            return '\n'.join(['=' * 120,
+                              f'CASE {self.subject_id}',
+                              f'\tPATIENT ID\t{self.patient_id}',
+                              f'\tSTUDY ID\t{self.study_id}\n',
+                              *self._log])
 
-        return True
 
-    def _write(self):
+class MHA2nnUNetConverter(Converter):
+    def __init__(
+        self,
+        output_dir: PathLike,
+        scans_dir: PathLike,
+        mha2nnunet_settings: Union[PathLike, Dict],
+        scans_out_dirname: str = 'imagesTr',
+        annotations_dir: Optional[PathLike] = None,
+        annotations_out_dirname: Optional[str] = 'labelsTr',
+    ):
+        """
+        Parameters
+        ----------
+        output_dir: PathLike
+            path to store the resulting nnUNet archive.
+        scans_dir: PathLike
+            directory name to store scans in, relative to `output_dir`.
+        scans_out_dirname: str, default: 'imagesTr'
+            dirname to store scan output, will be a direct descendant of `output_dir`.
+        annotations_dir: PathLike
+            path to the annotations archive. Used as base path for the relative paths of the archive items.
+        annotations_out_dirname: str, default: 'labelsTr'
+            dirname to store annotation output, will be a direct descendant of `output_dir`.
+        mha2nnunet_settings: Union[PathLike, Dict]
+            object with cases, nnUNet-shaped dataset.json and optional parameters.
+            May be a dictionary containing mappings, dataset.json, and optionally options, or a path to a JSON file with these elements.
+            - dataset_json: see nnU-Net's dataset conversion on details for the dataset.json file:
+                https://github.com/MIC-DKFZ/nnUNet/blob/master/documentation/dataset_conversion.md
+            - cases: list of objects. Each case should be an object with a patient_id,
+                study_id, relative paths to scans and optionally a path to an annotation
+            - options: (optional)
+                - num_threads: number of multithreading threads.
+                    Default: 4.
+                - verbose: control logfile verbosity. 0 does not output a logfile,
+                    1 logs cases which have critically failed, 2 logs all cases (may lead to
+                    very large log files)
+                    Default: 1
         """
-        Preprocess studies for nnUNet
+        if isinstance(mha2nnunet_settings, (Path, str)):
+            with open(mha2nnunet_settings) as fp:
+                mha2nnunet_settings = json.load(fp)
+
+        jsonschema.validate(mha2nnunet_settings, mha2nnunet_schema, cls=jsonschema.Draft7Validator)
+        self.settings = MHA2nnUNetSettings(
+            dataset_json=mha2nnunet_settings['dataset_json'],
+            preprocessing=PreprocessingSettings(**mha2nnunet_settings['preprocessing']),
+            **mha2nnunet_settings.get('options', {})
+        )
 
-        Requires:
-        - all_scan_properties: list of scan properties:
-        [
-            {
-                'input_path': path to input scan (T2/ADC/high b-value/etc.),
-                'output_path': path to store preprocessed scan,
-            }
-        ]
+        self.scans_dir = Path(scans_dir)
+        self.annotations_dir = Path(annotations_dir) if annotations_dir else None
 
-        - label_properties: label properties:
-        {
-            'input_path': path to label,
-            'output_path': path to store preprocessed label,
-        }
-        """
-        successes, errors = 0, 0
-
-        with ThreadPoolExecutor(max_workers=self.num_threads) as pool:
-            futures = {pool.submit(self._convert_item, item): item for item in self.valid_items}
-            for future in tqdm(as_completed(futures), total=self.num_valid_items):
-                item = futures[future]
-                try:
-                    success = future.result()
-                except Exception as e:
-                    item['error'] = f'Unexpected error: {e}'
-                    self.item_log(item, 'unexpected error')
-                    errors += 1
-                else:
-                    successes += 1 if success else 0
-
-        skipped = self.num_valid_items - successes - errors
-        self.info(f"Processed {plural(self.num_valid_items, 'item')}, "
-                  f"with {plural(skipped, 'case')} skipped and {plural(errors, 'error')}.")
-
-    def generate_json(self):
-        """
-        Create dataset.json for nnUNet raw data archive
-        """
-        if self.create_dataset_json:
-            # use contents of archive->dataset_json as starting point
-            json_dict = dict(self.dataset_json)
-            if 'name' not in json_dict:
-                json_dict['name'] = self.name
-            json_dict['numTraining'] = self.num_valid_items
-            json_dict['numTest'] = 0  # not implemented currently
-            json_dict['training'] = [
-                {
-                    "image": f"./imagesTr/{item['conversion_item'].subject_id}.nii.gz",
-                    "label": f"./labelsTr/{item['conversion_item'].subject_id}.nii.gz"
-                }
-                for item in self.valid_items
-            ]
-            json_dict['test'] = []
-
-            dataset_fn = self.output_dir / self.task / "dataset.json"
-            if not os.path.exists(dataset_fn):
-                self.info(f"Saving dataset info to {dataset_fn}")
-                with open(dataset_fn, 'w') as fp:
-                    json.dump(json_dict, fp, indent=4)
+        output_dir = Path(output_dir)
+        output_dir.mkdir(parents=True, exist_ok=True)
 
-    def convert(self):
-        """
-        Perform conversion steps:
-        1. Gather metadata
-        2. Preprocess and write scans
-        3. Generate dataset.json
-        """
-        for step in [self._plan, self._write]:
-            if self.has_valid_items:
-                step()
-                self.next_history()
-            else:
-                self.info("Aborted conversion, no items to convert.")
-
-        if len(self.items) == self.num_valid_items:
-            self.generate_json()
-        else:
-            self.info(f"Did not generate dataset.json, as only {self.num_valid_items}/{len(self.items)} items are converted successfully.")
+        self.scans_out_dir = output_dir / self.settings.task_name / scans_out_dirname
+        self.annotations_out_dir = output_dir / self.settings.task_name / annotations_out_dirname if annotations_dir else None
 
-        self.complete()
+        self.cases = self._init_cases(mha2nnunet_settings['archive'])
 
-    def item_log_value(self, item: Dict[str, str]):
-        lpad = ' ' * (len(str(len(self.items))) + 1)
+        self.initialize_log(output_dir, self.settings.verbose)
 
-        if 'error' in item:
-            result = f"{lpad}{item['error']}"
-        elif 'skip' in item:
-            result = f"{lpad}{item['skip']}"
-        else:
-            return None
+    def _init_cases(self, archive: List[Dict]) -> List[MHA2nnUNetCase]:
+        return [MHA2nnUNetCase(scans_dir=self.scans_dir, annotations_dir=self.annotations_dir, settings=self.settings, **kwargs) for kwargs in archive]
 
-        return f"{str(self.items.index(item)).zfill(len(lpad))} {item['patient_id']}/{item['study_id']}\n" \
-               f"\t{result}"
+    def convert(self):
+        self._convert(
+            title='MHA2nnUNet',
+            cases=self.cases,
+            parameters={
+                'scans_out_dir': self.scans_out_dir,
+                'annotations_out_dir': self.annotations_out_dir
+            },
+            num_threads=self.settings.num_threads,
+        )
 
-    @property
-    def task(self):
-        return self.dataset_json['task']
+    def prepare_dataset_paths(self):
+        """Prepare paths to scans and annotation in nnU-Net dataset.json format"""
+        return [
+            {
+                "image": f"./{self.scans_out_dir.name}/{case.subject_id}.nii.gz",
+                "label": f"./{self.annotations_out_dir.name}/{case.subject_id}.nii.gz"
+            }
+            for case in self.cases
+        ]
 
-    @property
-    def name(self):
-        if 'name' in self.dataset_json:
-            return self.dataset_json['name']
+    def create_dataset_json(self, path: PathLike = 'dataset.json', is_testset: bool = False) -> Dict:
+        """
+        Create dataset.json for nnUNet raw data archive.
+
+        Parameters
+        ----------
+        path: PathLike, default: nnU-Net raw data archive folder / task / dataset.json
+            path to save dataset info to. If None, will not output a file.
+        is_testset: bool, default: False
+            whether this conversation was a test set or a training set.
+
+        Returns
+        -------
+        dataset : dict
+            contents of dataset.json
+        """
+        if path is None:
+            return
+
+        dataset_path = self.scans_out_dir.parent / path
+        logging.info(f'Saving dataset info to {dataset_path}')
+
+        # use contents of archive->dataset_json as starting point
+        dataset_settings = self.settings.dataset_json
+        if 'name' not in dataset_settings:
+            dataset_settings['name'] = '_'.join(self.settings.task_name.split('_')[1:])
+
+        if is_testset:
+            dataset_settings["numTest"] = len(self.cases)
+            dataset_settings["test"] = self.prepare_dataset_paths()
+            if "numTraining" not in dataset_settings:
+                dataset_settings["numTraining"] = 0
+                dataset_settings["training"] = []
         else:
-            # grab e.g. Prostate_mpMRI_csPCa from Task101_Prostate_mpMRI_csPCa
-            return "_".join(self.task.split('_')[1:])
+            dataset_settings['numTraining'] = len(self.cases)
+            dataset_settings["training"] = self.prepare_dataset_paths()
+            if "numTest" not in dataset_settings:
+                dataset_settings["numTest"] = 0
+                dataset_settings["test"] = []
+
+        with open(dataset_path, 'w') as fp:
+            json.dump(dataset_settings, fp, indent=4)
+
+        return dataset_settings
diff --git a/src/picai_prep/utilities.py b/src/picai_prep/utilities.py
index ee913c7..91893d8 100644
--- a/src/picai_prep/utilities.py
+++ b/src/picai_prep/utilities.py
@@ -50,17 +50,6 @@ def make_sitk_readers() -> Tuple[sitk.ImageFileReader, sitk.ImageSeriesReader]:
     return file_reader, series_reader
 
 
-metadata_defaults = {
-    "patient_id": {
-        "key": "0010|0020",
-        "error": "No PatientID metadata key found and no custom 'patient_id' provided"
-    },
-    "study_id": {
-        "key": "0020|000d",
-        "error": "No StudyInstanceUID metadata key found and no custom 'study_id' provided"
-    },
-}
-
 __all__ = [
     # Explicitly expose these functions for easier imports
     "dcm2mha_schema",


Test patch:
diff --git a/tests/Development-README.md b/tests/Development-README.md
index f0024af..6e77484 100644
--- a/tests/Development-README.md
+++ b/tests/Development-README.md
@@ -21,4 +21,7 @@ Perform tests:
 pytest
 mypy src
 flake8 src
-```
\ No newline at end of file
+```
+
+# Programming conventions
+AutoPEP8 for formatting (this can be done automatically on save, see e.g. https://code.visualstudio.com/docs/python/editing)
diff --git a/tests/test_mha2nnunet.py b/tests/test_mha2nnunet.py
index 3bd296c..985afe8 100644
--- a/tests/test_mha2nnunet.py
+++ b/tests/test_mha2nnunet.py
@@ -59,12 +59,13 @@ def test_mha2nnunet(
 
     # convert MHA archive to nnUNet raw data
     archive = MHA2nnUNetConverter(
-        input_path=input_dir.as_posix(),
-        annotations_path=annotations_dir.as_posix(),
-        output_path=output_dir.as_posix(),
-        settings_path=settings_path,
+        output_dir=output_dir.as_posix(),
+        scans_dir=input_dir.as_posix(),
+        annotations_dir=annotations_dir.as_posix(),
+        mha2nnunet_settings=settings_path
     )
     archive.convert()
+    archive.create_dataset_json()
 
     # check dataset.json
     path_out = task_dir / "dataset.json"
@@ -149,18 +150,16 @@ def test_mha2nnunet_inference(
 
     # convert MHA archive to nnUNet raw data
     archive = MHA2nnUNetConverter(
-        input_path=input_dir.as_posix(),
-        output_path=output_dir.as_posix(),
-        out_dir_scans="imagesTs",
-        settings_path=settings_path
+        output_dir=output_dir.as_posix(),
+        scans_dir=input_dir.as_posix(),
+        scans_out_dirname='imagesTs',
+        mha2nnunet_settings=settings_path
     )
     archive.convert()
 
-    # check dataset.json
+    # for inference, no dataset.json should be created
     path_out = output_dir / task_name / "dataset.json"
-    path_out_expected = output_expected_dir / task_name / "dataset.json"
-    with open(path_out) as fp1, open(path_out_expected) as fp2:
-        assert json.load(fp1) == json.load(fp2)
+    assert not path_out.exists(), "dataset.json should not be generated in inference mode!"
 
     # compare output
     for subject_id in subject_list: