Read all tags instead of stopping before pixel data

.github/workflows/pythonpackage.yml

@@ -3,12 +3,16 @@ name: Python tests
 on: [push, pull_request]
 
 jobs:
-  build:
+  test:
     runs-on: ${{ matrix.os }}
     strategy:
+      fail-fast: false
       matrix:
-        os: [ubuntu-latest, macOS-latest, windows-2016]
-        python-version: [3.6, 3.7, 3.8, 3.9]
+        os: [ubuntu-latest, macOS-latest, windows-latest]
+        python-version: [3.6, 3.7, 3.8, 3.9, "3.10"]
+        exclude:
+          - os: macOS-latest
+            python-version: 3.6
     steps:
     - uses: actions/checkout@v1
     - name: Set up Python ${{ matrix.python-version }}

.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
 

README.md

@@ -2,12 +2,14 @@
 
 [![PyPI version](https://badge.fury.io/py/dicom-validator.svg)](https://pypi.org/project/dicom-validator) ![Python package](https://github.com/pydicom/dicom-validator/workflows/Python%20package/badge.svg) [![Python version](https://img.shields.io/pypi/pyversions/dicom-validator.svg)](https://pypi.org/project/dicom-validator)
 
-*dicom-validator* was planned to be a collection of simple pure python command line tools which get the input from 
-the DICOM standard in docbook format as provided by [ACR NEMA](http://medical.nema.org/). 
-As the only relevant tool is the validator tool, and no further tools are planned, the package has now been renamed
-from the original name *dcm-spec-tools*, and moved to the pydicom organization.
-
-Currently available tools:  
+*dicom-validator* was originally planned to be a collection of simple pure 
+python command line tools which get the input from the DICOM standard in 
+docbook format as provided by [ACR NEMA](http://medical.nema.org/). 
+As the only relevant tool is the validator tool, and no further tools are 
+planned, the package has now been renamed from the original name
+*dcm-spec-tools*, and moved to the pydicom organization.
+
+Available tools:  
 * `validate_iods` checks DICOM files for correct attributes for the given SOP class.  
 * `dump_dcm_info` outputs the DICOM tag IDs and values of a given DICOM file 
   (used as a simple demonstration for access to the DICOM standard).
@@ -36,10 +38,10 @@ Access to DICOM standard
 ------------------------
 
 Upon first start of a tool, part of the latest version of the DICOM standard
-in docbook format is downloaded, parsed, and the needed information saved in 
-json files. These files are then used by the tools. Periodically (once a 
-month), the tools check for a newer version of the DICOM standard and download 
-it if found.
+in docbook format (specifically, parts 3.3, 3.4 and 3.6) is downloaded, 
+parsed, and the needed information saved in json files.
+These files are then used by the tools. Periodically (once a month), the tools
+check for a newer version of the DICOM standard and download it if found.
 
 It is also possible to use older versions of the standard via a command line 
 option, provided they are still available for download (at the time of 
@@ -134,4 +136,4 @@ Tag (0040,A13A) (Referenced DateTime) is missing due to condition:
 ```
 
 *Note:* No guarantees are given for the correctness of the results. This is 
-pre-alpha software and mostly thought as a proof of concept.
+alpha-stage software and mostly thought as a proof of concept.

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