Fixed exception on parsing some older DICOM standard versions

.github/workflows/testsuite.yml

@@ -14,33 +14,33 @@ jobs:
       matrix:
         os: [ubuntu-latest, macOS-latest, windows-latest]
         python-version: ["3.8", "3.9", "3.10", "3.11", "3.12"]
-        dicom-version: ["2023c"]
     steps:
     - uses: actions/checkout@v4
     - name: Set up Python ${{ matrix.python-version }}
       uses: actions/setup-python@v5
       with:
         python-version: ${{ matrix.python-version }}
 
-    - name: Install dependencies
-      run: |
-        python -m pip install --upgrade pip
-        pip install -r requirements-dev.txt
-        pip install -e .
-
     - name: Cache DICOM standard
       id: cache-dicom
       uses: actions/cache@v4
       with:
         path: dicom_validator/tests/fixtures/standard
-        key: ${{ matrix.dicom-version }}
+        key: "2015b_2023c"
         enableCrossOsArchive: true
 
     - name: Download DICOM standard
       if: steps.cache-dicom.outputs.cache-hit != 'true'
       run: |
         pip install -e .
-        python .github/workflows/get_revision.py ${{ matrix.dicom-version }}  "`pwd`/dicom_validator/tests/fixtures/standard"
+        python .github/workflows/get_revision.py 2015b "`pwd`/dicom_validator/tests/fixtures/standard"
+        python .github/workflows/get_revision.py 2023c "`pwd`/dicom_validator/tests/fixtures/standard"
+
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements-dev.txt
+        pip install -e .
 
     - name: Run tests with coverage
       run: |

CHANGES.md

@@ -1,6 +1,11 @@
 # Dicom Validator Release Notes
 The released versions correspond to PyPi releases.
 
+## Unreleased
+
+### Fixes
+* fixed exception on parsing some older DICOM standard versions
+
 ## [Version 0.5.0](https://pypi.python.org/pypi/dicom-validator/0.5.0) (2024-01-25)
 Adds enum checks and fixes a regression.
 

dicom_validator/spec_reader/part3_reader.py

@@ -301,6 +301,8 @@ def _get_tag_name_and_level(
             # Pop off (delete) the last level_delta items.
             level_delta = current_level - level
             del current_descriptions[-level_delta:]
+            if not current_descriptions:
+                current_descriptions.append({})
         return tag_name, level
 
     def _collect_modules(self, table_section, check_rowspan):

dicom_validator/tests/spec_reader/conftest.py

@@ -24,23 +24,38 @@ def standard_path(fixture_path):
 
 
 @pytest.fixture(scope="session")
-def spec_fixture_path(standard_path):
-    yield standard_path / CURRENT_REVISION / "docbook"
+def revision(request):
+    if hasattr(request, "param"):
+        revision = request.param
+    else:
+        revision = CURRENT_REVISION
+    yield revision
 
 
 @pytest.fixture(scope="session")
-def dict_info(standard_path):
+def revision_path(standard_path, revision):
+    yield standard_path / revision
+
+
+@pytest.fixture(scope="session")
+def spec_fixture_path(revision_path):
+    yield revision_path / "docbook"
+
+
+@pytest.fixture(scope="session")
+def dict_info(revision_path):
     from dicom_validator.spec_reader.edition_reader import EditionReader
 
-    json_fixture_path = standard_path / CURRENT_REVISION / "json"
+    json_fixture_path = revision_path / "json"
     with open(json_fixture_path / EditionReader.dict_info_json) as info_file:
         info = json.load(info_file)
     yield info
 
 
 @pytest.fixture(scope="module")
 def spec_path(fs_module, spec_fixture_path):
-    fs_module.add_real_directory(spec_fixture_path)
+    if not fs_module.exists(spec_fixture_path):
+        fs_module.add_real_directory(spec_fixture_path)
     yield spec_fixture_path
 
 

dicom_validator/tests/spec_reader/test_part3_reader.py

@@ -1,6 +1,6 @@
-from xml.etree import ElementTree
 from pathlib import Path
 from unittest.mock import patch
+from xml.etree import ElementTree
 
 import pytest
 
@@ -48,24 +48,37 @@ def test_read_incomplete_doc_file(self, fs):
         with pytest.raises(SpecReaderParseError):
             reader.iod_description("A.6")
 
-    def test_lookup_sop_class(self, reader):
+    @pytest.mark.parametrize(
+        "revision,iod_name",
+        [("2015b", "Computed Tomography Image IOD"), ("2023c", "CT Image IOD")],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_lookup_sop_class(self, reader, iod_name):
         with pytest.raises(SpecReaderLookupError):
             reader.iod_description("A.0")
         description = reader.iod_description(chapter="A.3")
         assert description is not None
         assert "title" in description
-        assert description["title"] == "CT Image IOD"
-
-    def test_get_iod_modules(self, reader):
+        assert description["title"] == iod_name
+
+    @pytest.mark.parametrize(
+        "revision,module_nr",
+        [("2015b", 27), ("2023c", 28)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_get_iod_modules(self, reader, module_nr):
         description = reader.iod_description(chapter="A.38.1")
         assert "modules" in description
         modules = description["modules"]
-        assert len(modules) == 28
+        assert len(modules) == module_nr
         assert "General Equipment" in modules
         module = modules["General Equipment"]
         assert module["ref"] == "C.7.5.1"
         assert module["use"] == "M"
 
+    @pytest.mark.parametrize("revision", ["2015b", "2023c"], indirect=True)
     def test_optional_iod_module(self, reader):
         description = reader.iod_description(chapter="A.38.1")
         assert "modules" in description
@@ -75,18 +88,30 @@ def test_optional_iod_module(self, reader):
         assert module["ref"] == "C.7.1.3"
         assert module["use"] == "U"
 
-    def test_iod_descriptions(self, reader):
+    @pytest.mark.parametrize(
+        "revision,desc_nr",
+        [("2015b", 110), ("2023c", 151)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_iod_descriptions(self, reader, desc_nr):
         descriptions = reader.iod_descriptions()
-        assert len(descriptions) == 151
+        assert len(descriptions) == desc_nr
         assert "A.3" in descriptions
         assert "A.18" in descriptions
         assert "A.38.1" in descriptions
 
-    def test_group_macros(self, reader):
+    @pytest.mark.parametrize(
+        "revision,macro_nr",
+        [("2015b", 24), ("2023c", 27)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_group_macros(self, reader, macro_nr):
         descriptions = reader.iod_descriptions()
         assert not descriptions["A.3"]["group_macros"]
         enhanced_ct_macros = descriptions["A.38.1"]["group_macros"]
-        assert len(enhanced_ct_macros) == 27
+        assert len(enhanced_ct_macros) == macro_nr
         pixel_measures = enhanced_ct_macros.get("Pixel Measures")
         assert pixel_measures
         assert pixel_measures["ref"] == "C.7.6.16.2.1"
@@ -110,18 +135,30 @@ def test_group_macros(self, reader):
         assert condition.tag == "(0008,0008)"
         assert condition.values == ["ORIGINAL", "MIXED"]
 
-    def test_module_description(self, reader):
+    @pytest.mark.parametrize(
+        "revision,desc_nr",
+        [("2015b", 9), ("2023c", 9)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_module_description(self, reader, desc_nr):
         with pytest.raises(SpecReaderLookupError):
             reader.module_description("C.9.9.9")
         description = reader.module_description("C.7.1.3")
-        assert len(description) == 9
+        assert len(description) == desc_nr
         assert "(0012,0031)" in description
         assert description["(0012,0031)"]["name"] == "Clinical Trial Site Name"
         assert description["(0012,0031)"]["type"] == "2"
 
-    def test_sequence_inside_module_description(self, reader):
+    @pytest.mark.parametrize(
+        "revision,desc_nr",
+        [("2015b", 3), ("2023c", 6)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_sequence_inside_module_description(self, reader, desc_nr):
         description = reader.module_description("C.7.2.3")
-        assert len(description) == 6
+        assert len(description) == desc_nr
         assert "(0012,0083)" in description
         assert "items" in description["(0012,0083)"]
         sequence_description = description["(0012,0083)"]["items"]
@@ -144,17 +181,29 @@ def test_referenced_macro(self, reader):
         assert len(description) == 20
         assert "(0028,0103)" in description
 
-    def test_module_descriptions(self, reader):
+    @pytest.mark.parametrize(
+        "revision,desc_nr",
+        [("2015b", 451), ("2023c", 572)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_module_descriptions(self, reader, desc_nr):
         descriptions = reader.module_descriptions()
-        assert len(descriptions) == 572
-
-    def test_conditional_include_in_sr_module(self, reader):
+        assert len(descriptions) == desc_nr
+
+    @pytest.mark.parametrize(
+        "revision,include_nr",
+        [("2015b", 9), ("2023c", 10)],
+        indirect=["revision"],
+        scope="session",
+    )
+    def test_conditional_include_in_sr_module(self, reader, include_nr):
         description = reader.module_description("C.17.3")
         assert "include" in description
         assert "C.17-5" in [d["ref"] for d in description["include"]]
         description = reader.module_description("C.17-5")
         assert "include" in description
-        assert len(description["include"]) == 10
+        assert len(description["include"]) == include_nr
         # all macros are included conditionally
         assert all("cond" in d for d in description["include"])
 
@@ -176,6 +225,9 @@ def test_parsed_enum_values(self, reader):
         enums = description["(0082,0036)"]["enums"]
         assert enums == ["FAILURE", "WARNING", "INFORMATIVE"]
 
+    @pytest.mark.parametrize(
+        "revision", ["2015b", "2023c"], indirect=True, scope="session"
+    )
     def test_linked_enum_values(self, reader):
         description = reader.module_description("10.24")
         assert "(300A,0450)" in description  # Device Motion Control Sequence

dicom_validator/tests/test_cmdline_tools.py

@@ -6,8 +6,6 @@
 from dicom_validator.spec_reader.enum_parser import EnumParser
 from dicom_validator.validate_iods import main
 
-CURRENT_REVISION = "2023c"
-
 
 @pytest.fixture(scope="session")
 def fixture_path():
@@ -24,15 +22,18 @@ def dicom_fixture_path(fixture_path):
     yield fixture_path / "dicom"
 
 
-def test_validate_sr(caplog, standard_path, dicom_fixture_path):
+@pytest.mark.order(0)
+@pytest.mark.parametrize("revision", ["2015b", "2023c"])
+def test_validate_sr(revision, caplog, standard_path, dicom_fixture_path):
+    # test also for 2015b to test an issue causing an exception
     rtdose_path = dicom_fixture_path / "rtdose.dcm"
     # recreate json files to avoid getting the cached ones
     # relies on the fact that this test is run first
     cmd_line_args = [
         "-src",
         str(standard_path),
         "-r",
-        CURRENT_REVISION,
+        revision,
         "--recreate-json",
         str(rtdose_path),
     ]

requirements-dev.txt

@@ -2,3 +2,4 @@ pydicom>=2.3.0
 pyfakefs>=3.4.1
 pytest
 pytest-cov
+pytest-order