Read all tags instead of stopping before pixel data

- use defer size instead to avoid loading pixel data
- fixes #6

.landscape.yml

@@ -1,4 +1,3 @@
 doc-warnings: false
 test-warnings: true
-strictness: high
-max-line-length: 120
+strictness: high

.pylintrc

@@ -2,9 +2,6 @@
 # disable check C0111 for doc strings
 disable=C0111
 
-[FORMAT]
-max-line-length=120
-
 [BASIC]
 good-names=i,j,k,ex,_,setUp,setUpClass
 

dcm_spec_tools/dump_dcm_info.py

@@ -3,11 +3,10 @@
 """
 
 import argparse
-import json
 import os
 import re
 
-from pydicom import filereader
+from pydicom import dcmread
 from pydicom.errors import InvalidDicomError
 
 from dcm_spec_tools.spec_reader.edition_reader import EditionReader
@@ -116,7 +115,7 @@ def print_sequence(self, sequence):
     def dump_file(self, file_path):
         try:
             print('\n' + file_path)
-            dataset = filereader.read_file(
+            dataset = dcmread(
                 file_path, stop_before_pixels=self.show_image_data, force=True)
             self.print_dataset(dataset)
         except (InvalidDicomError, KeyError):
@@ -167,13 +166,8 @@ def main():
         return 1
 
     json_path = os.path.join(destination, 'json')
-    with open(os.path.join(json_path,
-                           edition_reader.dict_info_json)) as info_file:
-        dict_info = json.load(info_file)
-    with open(os.path.join(json_path,
-                           edition_reader.uid_info_json)) as info_file:
-        uid_info = json.load(info_file)
-
+    dict_info = EditionReader.load_dict_info(json_path)
+    uid_info = EditionReader.load_uid_info(json_path)
     dumper = DataElementDumper(dict_info, uid_info, args.max_value_len,
                                args.show_image_data,
                                args.show_tags)

dcm_spec_tools/spec_reader/edition_reader.py

@@ -168,6 +168,24 @@ def get_chapter(self, revision, chapter, destination, is_current):
                         file_path))
             return False
 
+    @staticmethod
+    def load_info(json_path, info_json):
+        with open(os.path.join(json_path,
+                               info_json)) as info_file:
+            return json.load(info_file)
+
+    @classmethod
+    def load_dict_info(cls, json_path):
+        return cls.load_info(json_path, cls.dict_info_json)
+
+    @classmethod
+    def load_uid_info(cls, json_path):
+        return cls.load_info(json_path, cls.uid_info_json)
+
+    @classmethod
+    def load_module_info(cls, json_path):
+        return cls.load_info(json_path, cls.module_info_json)
+
     @classmethod
     def json_files_exist(cls, json_path):
         for filename in (cls.dict_info_json, cls.module_info_json,

dcm_spec_tools/tag_tools.py

@@ -1,6 +1,7 @@
 def tag_name_from_id_string(tag_id_string, dict_info):
     if dict_info and tag_id_string in dict_info:
-        return '{} ({})'.format(tag_id_string, dict_info[tag_id_string]['name'])
+        return '{} ({})'.format(
+            tag_id_string, dict_info[tag_id_string]['name'])
     return tag_id_string
 
 

dcm_spec_tools/tests/test_utils.py

@@ -11,3 +11,7 @@ def spec_fixture_path():
 
 def json_fixture_path():
     return os.path.join(fixture_path(), 'json')
+
+
+def dicom_fixture_path():
+    return os.path.join(fixture_path(), 'dicom')

dcm_spec_tools/validate_iods.py

@@ -1,36 +1,37 @@
 import argparse
-import json
 import logging
 import os
 
-from dcm_spec_tools.validator.dicom_file_validator import DicomFileValidator
 from dcm_spec_tools.spec_reader.edition_reader import EditionReader
+from dcm_spec_tools.validator.dicom_file_validator import DicomFileValidator
 
 
 def validate(args, base_path):
     json_path = os.path.join(base_path, 'json')
-    with open(os.path.join(json_path, EditionReader.dict_info_json)) as info_file:
-        dict_info = json.load(info_file)
-    with open(os.path.join(json_path, EditionReader.iod_info_json)) as info_file:
-        iod_info = json.load(info_file)
-    with open(os.path.join(json_path, EditionReader.module_info_json)) as info_file:
-        module_info = json.load(info_file)
+    dict_info = EditionReader.load_dict_info(json_path)
+    iod_info = EditionReader.load_uid_info(json_path)
+    module_info = EditionReader.load_module_info(json_path)
     log_level = logging.DEBUG if args.verbose else logging.INFO
     validator = DicomFileValidator(iod_info, module_info, dict_info, log_level)
     error_nr = 0
     for dicom_path in args.dicomfiles:
-        error_nr += sum(len(error) for error in list(validator.validate(dicom_path).values()))
+        error_nr += sum(len(error) for error in
+                        list(validator.validate(dicom_path).values()))
     return error_nr
 
 
 def main():
     parser = argparse.ArgumentParser(
         description='Validates DICOM file IODs')
-    parser.add_argument('dicomfiles', help='Path(s) of DICOM files or directories to validate',
+    parser.add_argument('dicomfiles',
+                        help='Path(s) of DICOM files or directories '
+                             'to validate',
                         nargs='+')
     parser.add_argument('--standard-path', '-src',
-                        help='Base path with the DICOM specs in docbook and json format',
-                        default=os.path.join(os.path.expanduser("~"), 'dicom-validator'))
+                        help='Base path with the DICOM specs in docbook '
+                             'and json format',
+                        default=os.path.join(os.path.expanduser("~"),
+                                             'dicom-validator'))
     parser.add_argument('--revision', '-r',
                         help='Standard revision (e.g. "2014c"), year of '
                              'revision, "current" or "local" (latest '
@@ -43,7 +44,8 @@ def main():
     edition_reader = EditionReader(args.standard_path)
     destination = edition_reader.get_revision(args.revision)
     if destination is None:
-        print('Failed to get DICOM edition {} - aborting'.format(args.revision))
+        print(
+            'Failed to get DICOM edition {} - aborting'.format(args.revision))
         return 1
 
     return validate(args, destination)

dcm_spec_tools/validator/dicom_file_validator.py

@@ -2,14 +2,15 @@
 import os
 import sys
 
-from pydicom import filereader
+from pydicom import dcmread
 from pydicom.errors import InvalidDicomError
 
 from dcm_spec_tools.validator.iod_validator import IODValidator
 
 
 class DicomFileValidator(object):
-    def __init__(self, iod_info, module_info, dict_info=None, log_level=logging.INFO):
+    def __init__(self, iod_info, module_info, dict_info=None,
+                 log_level=logging.INFO):
         self._module_info = module_info
         self._iod_info = iod_info
         self._dict_info = dict_info
@@ -40,10 +41,11 @@ def validate_dir(self, dir_path):
     def validate_file(self, file_path):
         self.logger.info('\nProcessing DICOM file "%s"', file_path)
         try:
-            data_set = filereader.read_file(file_path, stop_before_pixels=True)
+            data_set = dcmread(file_path, defer_size=1024)
         except InvalidDicomError:
             return {file_path: {'fatal': 'Invalid DICOM file'}}
         return {
-            file_path: IODValidator(data_set, self._iod_info, self._module_info, self._dict_info,
+            file_path: IODValidator(data_set, self._iod_info,
+                                    self._module_info, self._dict_info,
                                     self.logger.level).validate()
         }

dcm_spec_tools/validator/iod_validator.py

@@ -3,7 +3,7 @@
 import sys
 
 from dcm_spec_tools.spec_reader.condition import Condition
-from dcm_spec_tools.tag_tools import tag_name_from_id_string, tag_name_from_id
+from dcm_spec_tools.tag_tools import tag_name_from_id
 
 
 class InvalidParameterError(Exception):
@@ -30,7 +30,8 @@ def validate(self):
         else:
             sop_class_uid = self._dataset.SOPClassUID
             if sop_class_uid not in self._iod_info:
-                self.errors['fatal'] = 'Unknown SOPClassUID (probably retired): ' + sop_class_uid
+                self.errors['fatal'] = (f'Unknown SOPClassUID '
+                                        f'(probably retired): {sop_class_uid}')
             else:
                 self._validate_sop_class(sop_class_uid)
         if 'fatal' in self.errors:
@@ -82,11 +83,13 @@ def _validate_module(self, module, module_name):
             for tag_id_string, attribute in module_info.items():
                 tag_id = self._tag_id(tag_id_string)
                 if tag_id in self._dataset:
-                    message = self._incorrect_tag_message(tag_id, 'not allowed', None)
+                    message = self._incorrect_tag_message(tag_id,
+                                                          'not allowed', None)
                     errors.setdefault(message, []).append(tag_id_string)
         else:
             for tag_id_string, attribute in module_info.items():
-                result = self._validate_attribute(self._tag_id(tag_id_string), attribute)
+                result = self._validate_attribute(self._tag_id(tag_id_string),
+                                                  attribute)
                 if result is not None:
                     errors.setdefault(result, []).append(tag_id_string)
         return errors
@@ -104,7 +107,8 @@ def _log_module_required(self, module_name, required, allowed,
         self.logger.debug(msg)
 
     def _incorrect_tag_message(self, tag_id, error_kind, condition_dict):
-        msg = 'Tag {} is {}'.format(tag_name_from_id(tag_id, self._dict_info), error_kind)
+        msg = 'Tag {} is {}'.format(tag_name_from_id(tag_id, self._dict_info),
+                                    error_kind)
         if condition_dict:
             condition = Condition.read_condition(condition_dict)
             if condition.type != 'U':
@@ -125,15 +129,18 @@ def _validate_attribute(self, tag_id, attribute):
         elif attribute_type in ('1C', '2C'):
             if 'cond' in attribute:
                 condition_dict = attribute['cond']
-                tag_required, tag_allowed = self._object_is_required_or_allowed(condition_dict)
+                tag_required, tag_allowed = (
+                    self._object_is_required_or_allowed(condition_dict)
+                )
             else:
                 tag_required, tag_allowed = False, True
         else:
             tag_required, tag_allowed = False, True
         error_kind = None
         if not has_tag and tag_required:
             error_kind = 'missing'
-        elif tag_required and value_required and self._dataset[tag_id].value is None:
+        elif (tag_required and value_required and
+              self._dataset[tag_id].value is None):
             error_kind = 'empty'
         elif has_tag and not tag_allowed:
             error_kind = 'not allowed'
@@ -142,7 +149,6 @@ def _validate_attribute(self, tag_id, attribute):
                                               condition_dict)
             return msg
 
-
     def _object_is_required_or_allowed(self, condition):
         if isinstance(condition, str):
             condition = json.loads(condition)
@@ -157,9 +163,11 @@ def _object_is_required_or_allowed(self, condition):
 
     def _composite_object_is_required(self, condition):
         if 'and' in condition:
-            required = all(self._composite_object_is_required(cond) for cond in condition['and'])
+            required = all(self._composite_object_is_required(cond)
+                           for cond in condition['and'])
         elif 'or' in condition:
-            required = any(self._composite_object_is_required(cond) for cond in condition['or'])
+            required = any(self._composite_object_is_required(cond)
+                           for cond in condition['or'])
         else:
             required = self._object_is_required(condition)
         return required
@@ -230,5 +238,6 @@ def _expanded_module_info(self, module_info):
                 module_info.update(self._get_module_info(ref))
             del module_info['include']
         if 'items' in module_info:
-            module_info['items'] = self._expanded_module_info(module_info['items'])
+            module_info['items'] = self._expanded_module_info(
+                module_info['items'])
         return module_info

dcm_spec_tools/validator/tests/test_dicom_file_validator.py

@@ -1,17 +1,19 @@
 import json
 import logging
 import os
+import unittest
 
 import pyfakefs.fake_filesystem_unittest
 from pydicom import write_file
 from pydicom.dataset import Dataset, FileDataset
 
 from dcm_spec_tools.spec_reader.edition_reader import EditionReader
-from dcm_spec_tools.tests.test_utils import json_fixture_path
+from dcm_spec_tools.tests.test_utils import json_fixture_path, \
+    dicom_fixture_path
 from dcm_spec_tools.validator.dicom_file_validator import DicomFileValidator
 
 
-class DicomFileValidatorTest(pyfakefs.fake_filesystem_unittest.TestCase):
+class DicomFileValidatorTestBase(unittest.TestCase):
     iod_info = None
     module_info = None
 
@@ -25,11 +27,19 @@ def setUpClass(cls):
             cls.module_info = json.load(info_file)
 
     def setUp(self):
-        super(DicomFileValidatorTest, self).setUp()
-        self.setUpPyfakefs()
         logging.disable(logging.CRITICAL)
         self.validator = DicomFileValidator(self.iod_info, self.module_info)
 
+
+class FakeDicomFileValidatorTest(DicomFileValidatorTestBase,
+                                 pyfakefs.fake_filesystem_unittest.TestCase):
+    iod_info = None
+    module_info = None
+
+    def setUp(self):
+        super(FakeDicomFileValidatorTest, self).setUp()
+        self.setUpPyfakefs()
+
     @staticmethod
     def create_metadata():
         metadata = Dataset()
@@ -93,3 +103,17 @@ def test_non_fatal_errors(self):
         self.assertEqual(1, len(error_dict))
         errors = error_dict['test']
         self.assertNotIn('fatal', errors)
+
+
+class RealDicomFileValidatorTest(DicomFileValidatorTestBase):
+
+    def test_that_pixeldata_is_read(self):
+        # regression test for #6
+        rtdose_path = os.path.join(dicom_fixture_path(), 'rtdose.dcm')
+        error_dict = self.validator.validate(rtdose_path)
+        self.assertEqual(1, len(error_dict))
+        results = error_dict[rtdose_path]
+        self.assertIn('RT Series', results)
+        self.assertIn('Tag (0008,1070) is missing', results['RT Series'])
+        # if PixelData is not read, RT Dose will show errors
+        self.assertNotIn('RT Dose', results)