From 680e61b5d509776f5b0669cc42c7ac889a151419 Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Mon, 29 Aug 2016 12:44:41 -0500 Subject: [PATCH 01/12] Support external tool to generate acquisition uid This is a POC implementation. Consider passing custom function for generation rather than assuming source dicoms are in dir named the same as the resulting zip file. Then the zip meta would be accurate as well. --- reaper/dcm.py | 1 - reaper/dicom_reaper.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index 1091e96..82c1131 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -42,7 +42,6 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None) arc_path = util.create_archive(arcdir_path, dir_name) metadata = util.object_metadata(dcm, timezone, os.path.basename(arc_path)) util.set_archive_metadata(arc_path, metadata) - shutil.rmtree(arcdir_path) metadata_map[arc_path] = metadata return metadata_map diff --git a/reaper/dicom_reaper.py b/reaper/dicom_reaper.py index 54a18d2..6619426 100644 --- a/reaper/dicom_reaper.py +++ b/reaper/dicom_reaper.py @@ -3,10 +3,13 @@ import os import logging import datetime +import re +import zipfile from . import dcm from . import scu from . import reaper +from subprocess import Popen, PIPE, STDOUT log = logging.getLogger('reaper.dicom') @@ -19,6 +22,7 @@ def __init__(self, options): self.scu = scu.SCU(options.get('host'), options.get('port'), options.get('return_port'), options.get('aet'), options.get('aec')) super(DicomReaper, self).__init__(self.scu.aec, options) self.anonymize = options.get('anonymize') + self.uid_ext_command = options.get('uid_ext_command') self.query_tags = {self.map_key: ''} if self.opt_key is not None: @@ -74,10 +78,40 @@ def reap(self, _id, item, tempdir): return None, {} if success and reap_cnt == item['state']['images']: metadata_map = dcm.pkg_series(_id, reapdir, self.map_key, self.opt_key, self.anonymize, self.timezone) + self._custom_acq_uid(metadata_map) return True, metadata_map else: return False, {} + def _custom_acq_uid(self, metadata_map): + if self.uid_ext_command is None: + return + for filepath, metadata in metadata_map.iteritems(): + print(filepath) + print(metadata) + dcm_dir = re.sub('\.zip$','',filepath) + + unzipped_file = [os.path.join(dcm_dir, filename) for filename in os.listdir(dcm_dir)][0] + + formatted_string = self.uid_ext_command.format(unzipped_file) + print(formatted_string) + + + arg_list = formatted_string.split() + + p = Popen(arg_list, + stdout=PIPE, + stderr=STDOUT) + out, _ = p.communicate() + + if p.returncode and p.returncode != 0: + log.error('Error with command. Return code = {0}'.format(p.returncode)) + raise RuntimeError(out) + print(out.rstrip()) + metadata['acquisition']['uid'] = out.rstrip() + print(metadata) + + def update_arg_parser(ap): # pylint: disable=missing-docstring @@ -88,6 +122,7 @@ def update_arg_parser(ap): ap.add_argument('aec', help='remote AE title') ap.add_argument('-A', '--no-anonymize', dest='anonymize', action='store_false', help='do not anonymize patient name and birthdate') + ap.add_argument('--uid-ext-command', dest='uid_ext_command', help='Command to execute against single source file to generate acquisition uid', default=None) return ap From 6174cab05a0348db752383f8e7fc8d6a7fb99107 Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Tue, 30 Aug 2016 08:03:26 -0500 Subject: [PATCH 02/12] Reduce size of testdata set --- test/test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.sh b/test/test.sh index 88eb1c8..d6c0727 100755 --- a/test/test.sh +++ b/test/test.sh @@ -29,7 +29,7 @@ RECEIVER_PID=$! # Fetch test data mkdir -p $TESTDATA_DIR if [ ! "$(ls -A $TESTDATA_DIR)" ]; then - curl -L https://github.com/scitran/testdata/archive/master.tar.gz | tar xz -C $TESTDATA_DIR --strip-components 1 + curl -L https://github.com/scitran/testdata/archive/reaper-ci.tar.gz | tar xz -C $TESTDATA_DIR --strip-components 1 fi From 6c6226247409e5740a1b13c577041b07b8c62b46 Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Tue, 30 Aug 2016 08:21:08 -0500 Subject: [PATCH 03/12] Rename testdata folder for CI usage --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 815c96e..db970ea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,7 @@ env: global: - DCMTK_VERSION="dcmtk-3.6.1_20150924" - DCMTK_DB_DIR="dcmtk_dicom_db" - - TESTDATA_DIR="testdata" + - TESTDATA_DIR="testdata-ci" - ORTHANC_VERSION="Orthanc-1.1.0" cache: From 40ddae0af80f0e12645a3937c6e2390e24e0e99a Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Tue, 30 Aug 2016 08:31:11 -0500 Subject: [PATCH 04/12] Crate CI specific folder for dicom scp index --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index db970ea..6be1e25 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,7 +6,7 @@ python: env: global: - DCMTK_VERSION="dcmtk-3.6.1_20150924" - - DCMTK_DB_DIR="dcmtk_dicom_db" + - DCMTK_DB_DIR="dcmtk_dicom_db-ci" - TESTDATA_DIR="testdata-ci" - ORTHANC_VERSION="Orthanc-1.1.0" From 115afed8197ebc7c1df9b04eb59966c52f576007 Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Tue, 30 Aug 2016 09:01:58 -0500 Subject: [PATCH 05/12] Resolve pylon and pep8 errors --- reaper/dcm.py | 1 - reaper/dicom_reaper.py | 16 ++++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index 82c1131..2307951 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -1,7 +1,6 @@ """SciTran Reaper DICOM utility functions""" import os -import shutil import hashlib import logging import datetime diff --git a/reaper/dicom_reaper.py b/reaper/dicom_reaper.py index 6619426..17a351e 100644 --- a/reaper/dicom_reaper.py +++ b/reaper/dicom_reaper.py @@ -4,7 +4,6 @@ import logging import datetime import re -import zipfile from . import dcm from . import scu @@ -89,30 +88,28 @@ def _custom_acq_uid(self, metadata_map): for filepath, metadata in metadata_map.iteritems(): print(filepath) print(metadata) - dcm_dir = re.sub('\.zip$','',filepath) + dcm_dir = re.sub(r'\.zip$', '', filepath) unzipped_file = [os.path.join(dcm_dir, filename) for filename in os.listdir(dcm_dir)][0] formatted_string = self.uid_ext_command.format(unzipped_file) print(formatted_string) - arg_list = formatted_string.split() p = Popen(arg_list, - stdout=PIPE, - stderr=STDOUT) + stdout=PIPE, + stderr=STDOUT) out, _ = p.communicate() if p.returncode and p.returncode != 0: - log.error('Error with command. Return code = {0}'.format(p.returncode)) + log.error('Error with command. Return code = %d', p.returncode) raise RuntimeError(out) print(out.rstrip()) metadata['acquisition']['uid'] = out.rstrip() print(metadata) - def update_arg_parser(ap): # pylint: disable=missing-docstring ap.add_argument('host', help='remote hostname or IP') @@ -122,7 +119,10 @@ def update_arg_parser(ap): ap.add_argument('aec', help='remote AE title') ap.add_argument('-A', '--no-anonymize', dest='anonymize', action='store_false', help='do not anonymize patient name and birthdate') - ap.add_argument('--uid-ext-command', dest='uid_ext_command', help='Command to execute against single source file to generate acquisition uid', default=None) + ap.add_argument('--uid-ext-command', + dest='uid_ext_command', + help='Command to execute against single source file to generate acquisition uid', + default=None) return ap From 20b824fa9f8f26a27f891aa60275726f6d5d8c16 Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Tue, 30 Aug 2016 09:29:48 -0500 Subject: [PATCH 06/12] Resolve pylon convention messages --- reaper/dicom_reaper.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/reaper/dicom_reaper.py b/reaper/dicom_reaper.py index 17a351e..ea08edb 100644 --- a/reaper/dicom_reaper.py +++ b/reaper/dicom_reaper.py @@ -4,11 +4,11 @@ import logging import datetime import re +from subprocess import Popen, PIPE, STDOUT from . import dcm from . import scu from . import reaper -from subprocess import Popen, PIPE, STDOUT log = logging.getLogger('reaper.dicom') @@ -86,28 +86,22 @@ def _custom_acq_uid(self, metadata_map): if self.uid_ext_command is None: return for filepath, metadata in metadata_map.iteritems(): - print(filepath) - print(metadata) dcm_dir = re.sub(r'\.zip$', '', filepath) unzipped_file = [os.path.join(dcm_dir, filename) for filename in os.listdir(dcm_dir)][0] formatted_string = self.uid_ext_command.format(unzipped_file) - print(formatted_string) - arg_list = formatted_string.split() - p = Popen(arg_list, - stdout=PIPE, - stderr=STDOUT) - out, _ = p.communicate() + proc = Popen(arg_list, + stdout=PIPE, + stderr=STDOUT) + out, _ = proc.communicate() - if p.returncode and p.returncode != 0: - log.error('Error with command. Return code = %d', p.returncode) + if proc.returncode and proc.returncode != 0: + log.error('Error with command. Return code = %d', proc.returncode) raise RuntimeError(out) - print(out.rstrip()) metadata['acquisition']['uid'] = out.rstrip() - print(metadata) def update_arg_parser(ap): From 60ce3c7e754e205bb835e1ad746394044b7ab698 Mon Sep 17 00:00:00 2001 From: Gunnar Schaefer Date: Mon, 19 Sep 2016 12:41:55 -0700 Subject: [PATCH 07/12] Add full custom metadata handling --- .travis.yml | 2 +- reaper/dcm.py | 27 +++++++++++++++++++++++---- reaper/dicom_reaper.py | 34 ++++------------------------------ reaper/util.py | 6 +++++- test/lint.sh | 4 ++-- 5 files changed, 35 insertions(+), 38 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6be1e25..85f84be 100644 --- a/.travis.yml +++ b/.travis.yml @@ -33,7 +33,7 @@ install: script: - ./test/test.sh - - ./test/lint.sh reaper + - ./test/lint.sh after_success: - ./docker/build-trigger.sh Tag "${TRAVIS_TAG}" "${BUILD_TRIGGER_URL}" diff --git a/reaper/dcm.py b/reaper/dcm.py index 2307951..20d778c 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -1,9 +1,11 @@ """SciTran Reaper DICOM utility functions""" -import os +import datetime import hashlib import logging -import datetime +import os +import shlex +import subprocess import dicom @@ -16,7 +18,15 @@ GEMS_TYPE_VXTL = ['DERIVED', 'SECONDARY', 'VXTL STATE'] -def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None): +def __external_metadata(command, filepath): + try: + return subprocess.check_output(shlex.split(command), filepath) + except subprocess.CalledProcessError as ex: + log.error('Error running external command. Exit %d', ex.returncode) + return None + + +def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, additional_metadata=None): # pylint: disable=missing-docstring dcm_dict = {} log.info('inspecting %s', _id) @@ -39,7 +49,15 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None) os.utime(filepath, (file_time, file_time)) # correct timestamps os.rename(filepath, '%s.dcm' % os.path.join(arcdir_path, filename)) arc_path = util.create_archive(arcdir_path, dir_name) - metadata = util.object_metadata(dcm, timezone, os.path.basename(arc_path)) + for md_group_info in (additional_metadata or {}).itervalues(): + for md_field, md_value in md_group_info.iteritems(): + if md_value.startswith('^'): # DICOM header value + md_group_info[md_field] = dcm.raw_header.get(md_value[1:], None) + elif md_value.startswith('@'): # external command + md_group_info[md_field] = __external_metadata(md_value[1:], filepath) + else: # verbatim value + md_group_info[md_field] = md_value[1:] + metadata = util.object_metadata(dcm, timezone, os.path.basename(arc_path), additional_metadata) util.set_archive_metadata(arc_path, metadata) metadata_map[arc_path] = metadata return metadata_map @@ -57,6 +75,7 @@ def __init__(self, filepath, map_key, opt_key=None, parse=False, anonymize=False if not parse and anonymize: raise Exception('Cannot anonymize DICOM file without parsing') dcm = dicom.read_file(filepath, stop_before_pixels=(not anonymize)) + self.raw_header = dcm self._id = dcm.get(map_key, '') self.opt = dcm.get(opt_key, '') if opt_key else None self.acq_no = str(dcm.get('AcquisitionNumber', '')) or None if dcm.get('Manufacturer').upper() != 'SIEMENS' else None diff --git a/reaper/dicom_reaper.py b/reaper/dicom_reaper.py index ea08edb..c53d13c 100644 --- a/reaper/dicom_reaper.py +++ b/reaper/dicom_reaper.py @@ -3,8 +3,6 @@ import os import logging import datetime -import re -from subprocess import Popen, PIPE, STDOUT from . import dcm from . import scu @@ -21,7 +19,8 @@ def __init__(self, options): self.scu = scu.SCU(options.get('host'), options.get('port'), options.get('return_port'), options.get('aet'), options.get('aec')) super(DicomReaper, self).__init__(self.scu.aec, options) self.anonymize = options.get('anonymize') - self.uid_ext_command = options.get('uid_ext_command') + self.additional_metadata = {group: {field: value} for group, field, value in options.get('metadata')} + print self.additional_metadata self.query_tags = {self.map_key: ''} if self.opt_key is not None: @@ -76,33 +75,11 @@ def reap(self, _id, item, tempdir): log.info('ignoring %s (non-matching opt-%s)', _id, self.opt) return None, {} if success and reap_cnt == item['state']['images']: - metadata_map = dcm.pkg_series(_id, reapdir, self.map_key, self.opt_key, self.anonymize, self.timezone) - self._custom_acq_uid(metadata_map) + metadata_map = dcm.pkg_series(_id, reapdir, self.map_key, self.opt_key, self.anonymize, self.timezone, self.additional_metadata) return True, metadata_map else: return False, {} - def _custom_acq_uid(self, metadata_map): - if self.uid_ext_command is None: - return - for filepath, metadata in metadata_map.iteritems(): - dcm_dir = re.sub(r'\.zip$', '', filepath) - - unzipped_file = [os.path.join(dcm_dir, filename) for filename in os.listdir(dcm_dir)][0] - - formatted_string = self.uid_ext_command.format(unzipped_file) - arg_list = formatted_string.split() - - proc = Popen(arg_list, - stdout=PIPE, - stderr=STDOUT) - out, _ = proc.communicate() - - if proc.returncode and proc.returncode != 0: - log.error('Error with command. Return code = %d', proc.returncode) - raise RuntimeError(out) - metadata['acquisition']['uid'] = out.rstrip() - def update_arg_parser(ap): # pylint: disable=missing-docstring @@ -113,10 +90,7 @@ def update_arg_parser(ap): ap.add_argument('aec', help='remote AE title') ap.add_argument('-A', '--no-anonymize', dest='anonymize', action='store_false', help='do not anonymize patient name and birthdate') - ap.add_argument('--uid-ext-command', - dest='uid_ext_command', - help='Command to execute against single source file to generate acquisition uid', - default=None) + ap.add_argument('--metadata', nargs=3, default=[], action='append', help='Additional metadata to package') return ap diff --git a/reaper/util.py b/reaper/util.py index 9558a03..7b5e872 100644 --- a/reaper/util.py +++ b/reaper/util.py @@ -61,7 +61,7 @@ def hrsize(size): return '%.0f%sB' % (size, 'Y') -def object_metadata(obj, timezone, filename): +def object_metadata(obj, timezone, filename, additional_metadata=None): # pylint: disable=missing-docstring metadata = { 'session': {'timezone': timezone}, @@ -75,6 +75,10 @@ def object_metadata(obj, timezone, filename): metadata['file']['name'] = filename metadata['session']['subject'] = metadata.pop('subject', {}) metadata['acquisition']['files'] = [metadata.pop('file', {})] + for md_group, md_group_info in (additional_metadata or {}).iteritems(): + metadata.setdefault(md_group, {}) + metadata[md_group].setdefault('metadata', {}) + metadata[md_group]['metadata'].update(md_group_info) return metadata diff --git a/test/lint.sh b/test/lint.sh index 5fdded5..325f641 100755 --- a/test/lint.sh +++ b/test/lint.sh @@ -6,9 +6,9 @@ unset CDPATH cd "$( dirname "${BASH_SOURCE[0]}" )/.." echo "Running pylint ..." -pylint --reports=no "$@" +pylint --reports=no reaper echo echo "Running pep8 ..." -pep8 --max-line-length=150 --ignore=E402 "$@" +pep8 --max-line-length=150 --ignore=E402 reaper From f6377ee470e931af9e359d2462c060484560db9a Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Tue, 20 Sep 2016 16:06:24 -0500 Subject: [PATCH 08/12] Resolve error with subprocess call --- reaper/dcm.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index 20d778c..63ed2dc 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -20,7 +20,10 @@ def __external_metadata(command, filepath): try: - return subprocess.check_output(shlex.split(command), filepath) + parms = shlex.split(command) + parms.append(filepath) + log.info(' command = %s', parms) + return subprocess.check_output(parms) except subprocess.CalledProcessError as ex: log.error('Error running external command. Exit %d', ex.returncode) return None From 51af51afbc30fc749aef4ff1c81f0060bdb95f02 Mon Sep 17 00:00:00 2001 From: Gunnar Schaefer Date: Tue, 20 Sep 2016 21:23:27 -0400 Subject: [PATCH 09/12] Polish subprocess invocation --- reaper/dcm.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index 63ed2dc..c4710ae 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -20,10 +20,9 @@ def __external_metadata(command, filepath): try: - parms = shlex.split(command) - parms.append(filepath) - log.info(' command = %s', parms) - return subprocess.check_output(parms) + args = shlex.split(command) + [filepath] + log.debug('External metadata cmd: %s', ' '.join(args)) + return subprocess.check_output(args) except subprocess.CalledProcessError as ex: log.error('Error running external command. Exit %d', ex.returncode) return None From c22d42d6af7f75eb74a74455997ec2c394091998 Mon Sep 17 00:00:00 2001 From: Ryan Sanford Date: Wed, 21 Sep 2016 10:22:27 -0500 Subject: [PATCH 10/12] Pass single file from series to external metadata cmd --- reaper/dcm.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index c4710ae..3418e9b 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -42,6 +42,7 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, dir_name = name_prefix + '.' + FILETYPE arcdir_path = os.path.join(path, '..', dir_name) os.mkdir(arcdir_path) + first_file = None for filepath in acq_paths: dcm = DicomFile(filepath, map_key, opt_key, parse=True, anonymize=anonymize, timezone=timezone) filename = os.path.basename(filepath) @@ -49,14 +50,17 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, filename = filename.replace('(none)', 'NA') file_time = max(int(dcm.acquisition_timestamp.strftime('%s')), 315561600) # zip can't handle < 1980 os.utime(filepath, (file_time, file_time)) # correct timestamps - os.rename(filepath, '%s.dcm' % os.path.join(arcdir_path, filename)) + new_filepath = '%s.dcm' % os.path.join(arcdir_path, filename) + os.rename(filepath, new_filepath) + if first_file is None: + first_file = new_filepath arc_path = util.create_archive(arcdir_path, dir_name) for md_group_info in (additional_metadata or {}).itervalues(): for md_field, md_value in md_group_info.iteritems(): if md_value.startswith('^'): # DICOM header value md_group_info[md_field] = dcm.raw_header.get(md_value[1:], None) elif md_value.startswith('@'): # external command - md_group_info[md_field] = __external_metadata(md_value[1:], filepath) + md_group_info[md_field] = __external_metadata(md_value[1:], first_file) else: # verbatim value md_group_info[md_field] = md_value[1:] metadata = util.object_metadata(dcm, timezone, os.path.basename(arc_path), additional_metadata) From 0fac345fafc89144ade4064e28a7690739ce1609 Mon Sep 17 00:00:00 2001 From: Gunnar Schaefer Date: Wed, 19 Oct 2016 16:00:01 -0700 Subject: [PATCH 11/12] Streamline external command invocation --- reaper/dcm.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index 3418e9b..490df11 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -42,7 +42,6 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, dir_name = name_prefix + '.' + FILETYPE arcdir_path = os.path.join(path, '..', dir_name) os.mkdir(arcdir_path) - first_file = None for filepath in acq_paths: dcm = DicomFile(filepath, map_key, opt_key, parse=True, anonymize=anonymize, timezone=timezone) filename = os.path.basename(filepath) @@ -50,17 +49,15 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, filename = filename.replace('(none)', 'NA') file_time = max(int(dcm.acquisition_timestamp.strftime('%s')), 315561600) # zip can't handle < 1980 os.utime(filepath, (file_time, file_time)) # correct timestamps - new_filepath = '%s.dcm' % os.path.join(arcdir_path, filename) - os.rename(filepath, new_filepath) - if first_file is None: - first_file = new_filepath + arc_filepath = '%s.dcm' % os.path.join(arcdir_path, filename) + os.rename(filepath, arc_filepath) arc_path = util.create_archive(arcdir_path, dir_name) for md_group_info in (additional_metadata or {}).itervalues(): for md_field, md_value in md_group_info.iteritems(): if md_value.startswith('^'): # DICOM header value md_group_info[md_field] = dcm.raw_header.get(md_value[1:], None) elif md_value.startswith('@'): # external command - md_group_info[md_field] = __external_metadata(md_value[1:], first_file) + md_group_info[md_field] = __external_metadata(md_value[1:], arc_filepath) else: # verbatim value md_group_info[md_field] = md_value[1:] metadata = util.object_metadata(dcm, timezone, os.path.basename(arc_path), additional_metadata) From 85e7d1f6450c4a2898f22bee19175a37aecbcbca Mon Sep 17 00:00:00 2001 From: Gunnar Schaefer Date: Wed, 19 Oct 2016 16:41:02 -0700 Subject: [PATCH 12/12] Add basic error handling to dicom reaper --- reaper/dcm.py | 23 ++++++++++++++++------- reaper/dicom_reaper.py | 14 +++++++++++--- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/reaper/dcm.py b/reaper/dcm.py index 490df11..047374a 100644 --- a/reaper/dcm.py +++ b/reaper/dcm.py @@ -18,14 +18,22 @@ GEMS_TYPE_VXTL = ['DERIVED', 'SECONDARY', 'VXTL STATE'] +class DicomError(Exception): + + """DicomError class""" + + pass + + def __external_metadata(command, filepath): try: args = shlex.split(command) + [filepath] log.debug('External metadata cmd: %s', ' '.join(args)) return subprocess.check_output(args) except subprocess.CalledProcessError as ex: - log.error('Error running external command. Exit %d', ex.returncode) - return None + msg = 'Error running external command. Exit code %d' % ex.returncode + log.error(msg) + raise DicomError(msg) def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, additional_metadata=None): @@ -68,16 +76,17 @@ def pkg_series(_id, path, map_key, opt_key=None, anonymize=False, timezone=None, class DicomFile(object): - """ - DicomFile class - """ + """DicomFile class""" # pylint: disable=too-few-public-methods def __init__(self, filepath, map_key, opt_key=None, parse=False, anonymize=False, timezone=None): if not parse and anonymize: - raise Exception('Cannot anonymize DICOM file without parsing') - dcm = dicom.read_file(filepath, stop_before_pixels=(not anonymize)) + raise DicomError('Cannot anonymize DICOM file without parsing') + try: + dcm = dicom.read_file(filepath, stop_before_pixels=(not anonymize)) + except: + raise DicomError() self.raw_header = dcm self._id = dcm.get(map_key, '') self.opt = dcm.get(opt_key, '') if opt_key else None diff --git a/reaper/dicom_reaper.py b/reaper/dicom_reaper.py index c53d13c..131dbab 100644 --- a/reaper/dicom_reaper.py +++ b/reaper/dicom_reaper.py @@ -57,6 +57,7 @@ def instrument_query(self): return i_state def reap(self, _id, item, tempdir): + # pylint: disable=too-many-return-statements if item['state']['images'] == 0: log.info('ignoring %s (zero images)', _id) return None, {} @@ -70,13 +71,20 @@ def reap(self, _id, item, tempdir): success, reap_cnt = self.scu.move(scu.SeriesQuery(SeriesInstanceUID=_id), reapdir) log.info('reaped %s (%d images) in %.1fs', _id, reap_cnt, (datetime.datetime.utcnow() - reap_start).total_seconds()) if success and reap_cnt > 0: - df = dcm.DicomFile(os.path.join(reapdir, os.listdir(reapdir)[0]), self.map_key, self.opt_key) + try: + df = dcm.DicomFile(os.path.join(reapdir, os.listdir(reapdir)[0]), self.map_key, self.opt_key) + except dcm.DicomError: + return False, {} if not self.is_desired_item(df.opt): log.info('ignoring %s (non-matching opt-%s)', _id, self.opt) return None, {} if success and reap_cnt == item['state']['images']: - metadata_map = dcm.pkg_series(_id, reapdir, self.map_key, self.opt_key, self.anonymize, self.timezone, self.additional_metadata) - return True, metadata_map + try: + metadata_map = dcm.pkg_series(_id, reapdir, self.map_key, self.opt_key, self.anonymize, self.timezone, self.additional_metadata) + except dcm.DicomError: + return False, {} + else: + return True, metadata_map else: return False, {}