Merge pull request Open-Minds-Lab#57 from sinhaharsh/master

Changes : Email and Testing on matrix.os

.gitattributes

@@ -1 +1,3 @@
 mrQA/_version.py export-subst
+.github/workflows/continuous-integration.yml merge=oursstrategy
+

.github/workflows/codacy.yml

@@ -33,6 +33,7 @@ jobs:
       actions: read # only required for a private repository by github/codeql-action/upload-sarif to get the Action run status
     name: Codacy Security Scan
     runs-on: ubuntu-latest
+    if: ${{ !github.event.act }} # skip during local actions testing
     steps:
       # Checkout the repository to the GitHub Actions runner
       - name: Checkout code

.github/workflows/continuous-integration.yml

@@ -1,6 +1,6 @@
 # This workflow will install Python dependencies, run tests and lint with a variety of Python versions
 # For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python
-
+# Workflow for master branch
 name: tests
 
 on:
@@ -16,7 +16,7 @@ jobs:
             fail-fast: false
             matrix:
                 os: [ubuntu-latest, macos-latest]
-                python-version: [ "3.8", "3.12" ]
+                python-version: [ "3.8", "3.9", "3.10","3.11", "3.12" ]
 
         steps:
             -   uses: actions/checkout@v3
@@ -27,17 +27,11 @@ jobs:
             -   name: Install dependencies
                 run: |
                     python -m pip install --upgrade pip
-                    pip install git+https://github.com/sinhaharsh/protocol.git#egg=protocol
-                    pip install git+https://github.com/sinhaharsh/MRdataset.git#egg=MRdataset
                     if [ -f requirements_dev.txt ]; then pip install -r requirements_dev.txt; fi
                     pip install .
             -   name: Lint with flake8
                 run: |
                     make lint
-                    # stop the build if there are Python syntax errors or undefined names
-                    # flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
-                    # exit-zero treats all errors as warnings. The GitHub editor is 127 chars wide
-                    # flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics
             -   name: Test with pytest
                 run: |
                     pytest

.gitignore

@@ -108,6 +108,9 @@ ENV/
 # codacy
 *.sarif
 
+# act
+event.json
+
 # mri protocol
 *.xml
 *.secrets

Makefile

@@ -66,7 +66,7 @@ coverage: ## check code coverage quickly with the default Python
 	$(BROWSER) htmlcov/index.html
 
 act:
-	act --secret-file .secrets
+	act --secret-file .secrets -e event.json
 
 docs: ## generate Sphinx HTML documentation, including API docs
 	$(MAKE) -C docs clean

examples/monitor_project.py

@@ -1,13 +1,18 @@
 import argparse
 import multiprocessing as mp
+import os
+import sys
+from datetime import datetime, timedelta
 from pathlib import Path
 
 from MRdataset import DatasetEmptyException, valid_dirs, load_mr_dataset
 from mrQA import monitor, logger, check_compliance
-from mrQA.utils import txt2list, filter_epi_fmap_pairs
+from mrQA.config import PATH_CONFIG, status_fpath
+from mrQA.utils import txt2list, log_latest_non_compliance, is_writable, \
+    send_email
 
 
-def main():
+def get_parser():
     """Console script for mrQA."""
     parser = argparse.ArgumentParser(
         description='Protocol Compliance of MRI scans',
@@ -20,15 +25,22 @@ def main():
     # Add help
     required.add_argument('-d', '--data-root', type=str, required=True,
                           help='A folder which contains projects'
-                               'to process')
+                               'to process. Required if task is monitor')
     optional.add_argument('-t', '--task', type=str,
                           help='specify the task to be performed, one of'
                                ' [monitor, compile]', default='monitor')
     optional.add_argument('-a', '--audit', type=str,
                           help='specify the audit type if compiling reports. '
                                'Choose one of [hz, vt]. Required if task is '
                                'compile',
-                          default='vt')
+                          default='hz')
+    optional.add_argument('-i', '--input-dir', type=str,
+                          help='specify the directory where the reports'
+                               ' are saved. Required if task is compile')
+    optional.add_argument('--date', type=str,
+                          help='compile all non-compliant subjects scanned '
+                               'after this date. Format: MM_DD_YYYY. Required'
+                               ' if task is compile')
     optional.add_argument('-o', '--output-dir', type=str,
                           default='/home/mrqa/mrqa_reports/',
                           help='specify the directory where the report'
@@ -39,105 +51,175 @@ def main():
                                'monitoring')
     required.add_argument('--config', type=str,
                           help='path to config file')
+    optional.add_argument('-e', '--email-config-path', type=str,
+                          help='filepath to email config file')
 
-    args = parser.parse_args()
-    if Path(args.data_root).exists():
-        data_root = Path(args.data_root)
-        non_empty_folders = []
-        for folder in data_root.iterdir():
-            if folder.is_dir() and any(folder.iterdir()):
-                non_empty_folders.append(folder)
-    else:
-        raise ValueError("Need a valid path to a folder, which consists of "
-                         f"projects to process. "
-                         f"Got {args.data_root}")
+    if len(sys.argv) < 2:
+        logger.critical('Too few arguments!')
+        parser.print_help()
+        parser.exit(1)
+
+    return parser
 
-    dirs = valid_dirs(non_empty_folders)
 
-    if len(non_empty_folders) < 2:
-        dirs = [dirs]
+def parse_args():
+    """Parse command line arguments."""
+    parser = get_parser()
+    args = parser.parse_args()
+    dirs = []
+
     if args.exclude_fpath is not None:
-        if not Path(args.exclude_fpath).exists():
+        if not Path(args.exclude_fpath).is_file():
             raise FileNotFoundError("Need a valid filepath to the exclude list")
         exclude_filepath = Path(args.exclude_fpath).resolve()
         skip_list = [Path(i).resolve() for i in txt2list(exclude_filepath)]
+    else:
+        skip_list = []
+
+    if args.task == 'monitor':
+        if Path(args.data_root).is_dir():
+            data_root = Path(args.data_root)
+            non_empty_folders = []
+            for folder in data_root.iterdir():
+                if folder.is_dir() and any(folder.iterdir()):
+                    non_empty_folders.append(folder)
+        else:
+            raise ValueError("Need a valid path to a folder, which consists of "
+                             f"projects to process. "
+                             f"Got {args.data_root}")
+
+        dirs = valid_dirs(non_empty_folders)
+
+        if len(non_empty_folders) < 2:
+            # If there is only one project, then the cast it into a list
+            dirs = [dirs]
+
+    elif args.task == 'compile':
+        if Path(args.input_dir).is_dir():
+            dirs = valid_dirs(Path(args.input_dir))
+
+        if args.date is None:
+            two_weeks_ago = datetime.now() - timedelta(days=14)
+            args.date = two_weeks_ago.strftime('%m_%d_%Y')
+        else:
+            try:
+                datetime.strptime(args.date, '%m_%d_%Y')
+            except ValueError:
+                raise ValueError("Incorrect date format, should be MM_DD_YYYY")
+
+    else:
+        raise NotImplementedError(f"Task {args.task} not implemented. Choose "
+                                  "one of [monitor, compile]")
+
+    if args.audit not in ['hz', 'vt']:
+        raise ValueError(f"Invalid audit type {args.audit}. Choose one of "
+                         f"[hz, vt]")
+
+    if args.output_dir is None:
+        logger.info('Use --output-dir to specify dir for final directory. '
+                    'Using default')
+        args.output_dir = PATH_CONFIG['output_dir'] / args.name.lower()
+        args.output_dir.mkdir(exist_ok=True, parents=True)
+    else:
+        if not Path(args.output_dir).is_dir():
+            try:
+                Path(args.output_dir).mkdir(parents=True, exist_ok=True)
+            except OSError as exc:
+                logger.error(
+                    f'Unable to create folder {args.output_dir} for '
+                    f'saving reports')
+                raise exc
+    if not is_writable(args.output_dir):
+        raise OSError(f'Output Folder {args.output_dir} is not writable')
+
+    for fpath in dirs:
+        if Path(fpath).resolve() in skip_list:
+            dirs.remove(fpath)
+
+    if not Path(args.config).is_file():
+        raise FileNotFoundError(
+            f'Expected valid file for config,  Got {args.config}'
+            f'the file does not exist')
+    if args.email_config_path:
+        if not Path(args.email_config_path).is_file():
+            raise FileNotFoundError(
+                f'Expected valid file for config_path,  Got {args.config_path}'
+                f'the file does not exist')
+    else:
+        logger.info('Use --email-config-path to specify filepath to email. '
+                    'Skipping')
+    return args, dirs
+
+
+def main():
+    """Console script for mrQA monitor project."""
+    args, dirs = parse_args()
 
-        for fpath in dirs:
-            if Path(fpath).resolve() in skip_list:
-                dirs.remove(fpath)
     if args.task == 'monitor':
         pool = mp.Pool(processes=10)
-        arguments = [(f, args.output_dir, args.config) for f in dirs]
-        pool.starmap(run, arguments)
+        arguments = [(f, args.output_dir, args.config,
+                      args.email_config_path) for f in dirs]
+        pool.starmap(run_monitor, arguments)
     elif args.task == 'compile':
-        compile_reports(args.data_root, args.output_dir, args.config,
-                        args.audit)
+        compile_reports(folder_paths=dirs, output_dir=args.output_dir,
+                        config_path=args.config,
+                        date=args.date, audit=args.audit)
     else:
         raise NotImplementedError(f"Task {args.task} not implemented. Choose "
                                   "one of [monitor, compile]")
 
 
-def run(folder_path, output_dir, config_path):
+def run_monitor(folder_path, output_dir, config_path, email_config_path=None):
+    """Run monitor for a single project"""
     name = Path(folder_path).stem
     print(f"\nProcessing {name}\n")
     output_folder = Path(output_dir) / name
     try:
-        monitor(name=name,
-                data_source=folder_path,
-                output_dir=output_folder,
-                decimals=2,
-                verbose=False,
-                ds_format='dicom',
-                tolerance=0,
-                config_path=config_path,
-                )
+        hz_flag, vt_flag, report_path = monitor(name=name,
+                                                data_source=folder_path,
+                                                output_dir=output_folder,
+                                                decimals=2,
+                                                verbose=False,
+                                                ds_format='dicom',
+                                                tolerance=0,
+                                                config_path=config_path,
+                                                )
+        if hz_flag:
+            log_fpath = status_fpath(output_dir, audit='hz')
+            logger.info(f"Non-compliant scans found for {name}")
+            logger.info(f"Check {log_fpath} for horizontal audit")
+            send_email(log_fpath, project_code=name, report_path=report_path,
+                       email_config=email_config_path)
     except DatasetEmptyException as e:
         logger.warning(f'{e}: Folder {name} has no DICOM files.')
 
 
-def compile_reports(folder_path, output_dir, config_path, audit='vt'):
+def compile_reports(folder_paths, output_dir, config_path, audit='hz',
+                    date=None):
+    """Compile reports for all projects in the folder_paths"""
     output_dir = Path(output_dir)
     complete_log = []
     # Look for all mrds.pkl file in the output_dir. For ex, mrqa_reports
     # Collect mrds.pkl files for all projects
-    mrds_files = list(Path(folder_path).rglob('*.mrds.pkl'))
-    if not mrds_files:
-        raise FileNotFoundError(f"No .mrds.pkl files found in {folder_path}")
-
-    for mrds in mrds_files:
-        ds = load_mr_dataset(mrds)
+    for sub_folder in folder_paths:
+        mrds_files = list(Path(sub_folder).rglob('*.mrds.pkl'))
+        if not mrds_files:
+            continue
+        latest_mrds = max(mrds_files, key=os.path.getctime)
+        # for mrds in mrds_files:
+        ds = load_mr_dataset(latest_mrds)
         # TODO : check compliance, but maybe its better is to save
         #  compliance results which can be re-used here
-        hz, vt = check_compliance(
+        hz_audit_results, vt_audit_results = check_compliance(
             ds,
             output_dir=output_dir / 'compiled_reports',
             config_path=config_path,
         )
-        if audit == 'hz':
-            non_compliant_ds = hz['non_compliant']
-            filter_fn = None
-            nc_params = ['ReceiveCoilActiveElements']
-            supplementary_params = ['BodyPartExamined']
-        elif audit == 'vt':
-            non_compliant_ds = vt['non_compliant']
-            nc_params = ['ShimSetting', 'PixelSpacing']
-            supplementary_params = []
-            # TODO: discuss what parameters can be compared between anatomical
-            #   and functional scans
-            # after checking compliance just look for epi-fmap pairs for now
-            filter_fn = filter_epi_fmap_pairs
-        else:
-            raise ValueError(f"Invalid audit type {audit}. Choose one of "
-                             f"[hz, vt]")
-
-        nc_log = non_compliant_ds.generate_nc_log(
-            parameters=nc_params,
-            suppl_params=supplementary_params,
-            filter_fn=filter_fn,
-            output_dir=output_dir,
-            audit=audit,
-            verbosity=4)
-
+        log_latest_non_compliance(dataset=hz_audit_results['non_compliant'],
+                                  config_path=config_path,
+                                  output_dir=output_dir / sub_folder.stem,
+                                  audit=audit,
+                                  date=date)
 
 
 if __name__ == "__main__":