Merge pull request #359 from DESm1th/update_errors

[ENH] Add better error messages + filter out noisy redcap messages

bin/dm_link.py

@@ -221,8 +221,11 @@ def link_archive(archive_path, dicom_path, scanid_field, config):
     target = os.path.join(dicom_path, scanid)
     target = target + datman.utils.get_extension(archive_path)
     if os.path.exists(target):
-        logger.error("Target: {} already exists for archive: {}"
-                     .format(target, archive_path))
+        logger.error(
+            f"Target path {target} for archive {archive_path} is already in "
+            f"use by another zip file ({os.path.realpath(target)}). "
+            "Please investigate."
+        )
         return
 
     relpath = os.path.relpath(archive_path, dicom_path)

bin/dm_redcap_scan_completed.py

@@ -6,15 +6,19 @@
     dm_redcap_scan_completed.py [options] <study>
 
 Arguments:
-    <study>             Name of the study to process
+    <study>                 Name of the study to process
 
 Options:
-    -q --quiet          Less logging
-    -v --verbose        Verbose logging
-    -d --debug          Debug logging
+    --ignore <regex>        An optional regex used to ignore records with
+                            a matching subject ID field. Regex must follow the
+                            python 're' library rules.
+    -q --quiet              Less logging
+    -v --verbose            Verbose logging
+    -d --debug              Debug logging
 """
 
 import os
+import re
 import sys
 import requests
 import logging
@@ -179,6 +183,7 @@ def main():
 
     arguments = docopt(__doc__)
     study = arguments['<study>']
+    ignore_re = arguments['--ignore']
     quiet = arguments['--quiet']
     verbose = arguments['--verbose']
     debug = arguments['--debug']
@@ -240,6 +245,11 @@ def main():
         # only grab records where instrument has been marked complete
         if not (item[date_field] and item[status_field] in status_val):
             continue
+        # Filter out records when the subject ID matches an optional regex
+        if ignore_re and re.match(ignore_re,
+                                  item[get_setting('RedcapSubj',
+                                                   default='par_id')]):
+            continue
         project_records.append(item)
 
     for record in project_records:

bin/dm_task_files.py

@@ -11,6 +11,7 @@
 import re
 import glob
 import logging
+from string import printable
 
 from docopt import docopt
 
@@ -137,7 +138,8 @@ def resolve_duplicate_names(task_files):
         file_paths = all_fnames[unique_name]
 
         if len(file_paths) == 1:
-            resolved_names[unique_name] = file_paths[0]
+            dest_name = strip_unprintable_chars(unique_name)
+            resolved_names[dest_name] = file_paths[0]
             continue
 
         common_prefix = os.path.commonprefix(file_paths)
@@ -156,6 +158,27 @@ def sort_fnames(file_list):
     return all_fnames
 
 
+def strip_unprintable_chars(path):
+    path_prefix, fname = os.path.split(path)
+
+    if is_printable(fname):
+        return path
+
+    new_fname = fname
+    for char in get_unprintable_chars(fname):
+        new_fname = new_fname.replace(char, "")
+
+    return os.path.join(path_prefix, new_fname)
+
+
+def is_printable(fname):
+    return get_unprintable_chars(fname) == set()
+
+
+def get_unprintable_chars(item):
+    return set(item).difference(printable)
+
+
 def morph_name(file_path, common_prefix):
     """
     Returns a unique name by finding the unique part of a file's path and
@@ -169,7 +192,7 @@ def morph_name(file_path, common_prefix):
         # Common prefix may have split a directory name, so derive the new name
         # from the original path instead to ensure full names are used
         new_name = "-".join(file_path.split("/")[-(dir_levels + 1) :])
-    return new_name
+    return strip_unprintable_chars(new_name)
 
 
 def add_to_dashboard(session, task_file):

datman/xnat.py

@@ -1706,6 +1706,14 @@ def download(self, xnat_conn, output_dir):
                          f" series {self.series}. Reason - {e}")
             return False
 
+        if os.path.getsize(dicom_zip) == 0:
+            logger.error(
+                f"Server returned an empty file for series {self.series} in "
+                f"session {self.experiment}. This may be a server error."
+            )
+            os.remove(dicom_zip)
+            return False
+
         logger.info(f"Unpacking archive {dicom_zip}")
 
         try: