Data source processing improvements (#3)

Author: scaramallion

.github/workflows/cron-check-to-pr.yml

@@ -0,0 +1,44 @@
+name: check-pull-request
+
+on:
+  schedule:
+    - cron: '48 22 * * 0'
+
+jobs:
+
+  check:
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+    env:
+      PACKAGE_UPDATED: "False"
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python ${{ matrix.python-version }}
+      uses: actions/setup-python@v2
+      with:
+        python-version: 3.9
+    - name: Install dependencies
+      run: |
+        pip install -U pip
+    - name: Download and check source data
+      run: |
+        pip install -e .[dev]
+        mkdir sr/temp
+        python sr/main.py
+    - name: Blacken code, run tests and get diff
+      if: ${{ env.PACKAGE_UPDATED == 'True' }}
+      run: |
+        black sr/*py
+        black sr/tables/*.py
+        pytest
+    - name: Create PR
+      if: ${{ env.PACKAGE_UPDATED == 'True' }}
+      uses: peter-evans/create-pull-request@v3
+      with:
+        committer: scaramallion <[email protected]>
+        author: scaramallion <[email protected]>
+        delete-branch: true
+        commit-message: "Update SR package"
+        title: "Update SR package"
+        base: master

.github/workflows/check-pr-release.yml .github/workflows/pr-test.yml

@@ -1,35 +1,32 @@
-name: check-pull-request
+name: pull-request-test
 
 on:
-#  schedule:
-#    - cron: '48 22 * * 0'
   pull_request:
     branches: [ master ]
 
-
 jobs:
 
-  check-pr:
+  check:
     runs-on: ubuntu-latest
     timeout-minutes: 30
+    env:
+      PACKAGE_UPDATED: "False"
 
     steps:
     - uses: actions/checkout@v2
     - name: Set up Python ${{ matrix.python-version }}
       uses: actions/setup-python@v2
       with:
         python-version: 3.9
+    - name: Install dependencies
+      run: |
+        pip install -U pip
     - name: Download and check source data
       run: |
         pip install -e .[dev]
         mkdir sr/temp
-        python sr/main.py --dev --force-regeneration
-        black sr/*py
-        black sr/tables/*.py
-        git diff --output=gitdiff.txt
-    - name: Upload artifacts
-      if: ${{ success() }}
-      uses: actions/upload-artifact@v2
-      with:
-        name: diff
-        path: gitdiff.txt
+        python sr/main.py --force-regeneration
+    - name: Run tests
+      if: ${{ env.PACKAGE_UPDATED == 'True' }}
+      run: |
+        pytest

.gitignore

@@ -25,3 +25,7 @@ build/*
 distribute*.egg
 distribute*.tar.gz
 __pycache__
+
+# Other
+.mypy_cache
+.pytest_cache

mypy.ini

@@ -1,6 +1,6 @@
 [mypy]
 python_version = 3.9
-exclude = sr/(tables|temp)
+exclude = sr/(temp|test)
 files = sr/
 show_error_codes = True
 warn_redundant_casts = True

setup.py

@@ -54,9 +54,10 @@ def get_version() -> str:
             "black==21.6b0",
             "mypy==0.902",
             "types-requests==0.1.11",
+            "pytest==6.2.4",
         ]
     },
     entry_points={
-        "pydicom.data.external_sources": "pydicom-data-sr = srdata:DataStore",
+        "pydicom.data.sr": "pydicom-data-sr = sr:foo",
     },
 )

sr/__init__.py

@@ -1,6 +1,9 @@
 from pathlib import Path
+from types import ModuleType
+from typing import Tuple
 
 from sr._version import __version__
+from sr.tables import _cid, _concepts, _snomed
 
 
 PACKAGE_DIR = Path(__file__).parent.resolve(strict=True)
@@ -11,3 +14,7 @@
 CID_FILE = SR_TABLES / "_cid.py"
 CONCEPTS_FILE = SR_TABLES / "_concepts.py"
 SNOMED_FILE = SR_TABLES / "_snomed.py"
+
+
+def foo() -> Tuple[ModuleType, ModuleType, ModuleType]:
+    return _cid, _concepts, _snomed

sr/_version.py

@@ -1,2 +1,2 @@
-__version__: str = "2021.06.16"
+__version__: str = "2021.06.19"
 __dicom_version__: str = "2021b"

sr/hashes.json

@@ -1238,4 +1238,4 @@
     "CID_9570.json": "9271628dc86f9919b296607639be7d7c",
     "part16_d1.html": "0555239bf8a9593c13b9af73ba9a2a2a",
     "part16_o1.html": "a811317afc72f15c17699de6134b941f"
-}
+}

sr/main.py

@@ -1,7 +1,9 @@
 import argparse
 from datetime import datetime
 import logging
+import os
 from pathlib import Path
+import subprocess
 from tempfile import TemporaryDirectory
 import time
 from typing import Optional, List, Tuple, Dict, Any
@@ -26,7 +28,7 @@
 )
 from sr.utils import (
     download_cid_files,
-    download_file,
+    download_files,
     compare_checksums,
     calculate_checksums,
 )
@@ -41,8 +43,6 @@
 TABLE_O1 = PART_16 + "/chapter_O.html"
 TABLE_D1 = PART_16 + "/chapter_D.html"
 
-WORKERS = 32
-
 
 def run(
     src: Optional[Path] = None,
@@ -86,9 +86,11 @@ def run(
         attempts = 0
         while attempts < 5:
             try:
-                download_cid_files((CID_HOST, CID_PATH), src, WORKERS)
-                download_file(TABLE_D1, src / "part16_d1.html")
-                download_file(TABLE_O1, src / "part16_o1.html")
+                download_cid_files((CID_HOST, CID_PATH), src)
+                download_files(
+                    [TABLE_D1, TABLE_O1],
+                    [src / "part16_d1.html", src / "part16_o1.html"],
+                )
                 break
             except TimeoutError as exc:
                 LOGGER.exception(exc)
@@ -102,12 +104,16 @@ def run(
     paths = sorted(cid_paths + table_paths)
 
     # 1. Compare the data in `src` against the reference hashes
-    if compare_checksums(paths, HASH_FILE) and not force_regeneration:
+    checksums_match = compare_checksums(paths, HASH_FILE)
+    if checksums_match and not force_regeneration:
         LOGGER.info("No change in source data found, exiting...")
         return False
 
     # 2. Source data has changed, regenerate the tables and update the package
-    LOGGER.info("Source data has changed - updating package")
+    if not checksums_match:
+        LOGGER.info("Source data has changed - updating package")
+    else:
+        LOGGER.debug("'--force-regeneration' used - updating package")
 
     table_o1 = src / "part16_o1.html"
     table_d1 = src / "part16_d1.html"
@@ -153,7 +159,7 @@ def write_hash_file(paths: List[Path]) -> None:
         else:
             LOGGER.warning("No checksums available to write to 'hashes.json'")
 
-        f.write("}")
+        f.write("}\n")
 
     LOGGER.info(f"'hashes.json' written with {len(checksums)} entries")
 
@@ -182,10 +188,13 @@ def write_snomed_file(codes: List[Tuple[str, str, str]]) -> None:
                 "#   'SRT': {snomed_id1: concept_id1, snomed_id2: ...},\n",
                 "# }\n",
                 "\n",
+                "from typing import Dict\n",
+                "\n\n",
             ]
         )
 
-        f.write("mapping = {}\n")
+        f.write("mapping: Dict[str, Dict[str, str]] = {}\n")
+
         # Write the SCT to SRT mappings
         f.write("\nmapping['SCT'] = {\n")
         for sct, srt, meaning in sorted(codes, key=lambda x: int(x[0])):
@@ -229,10 +238,12 @@ def write_cid_file(cid_lists: CIDListType, name_for_cid: NameForCIDType) -> None
                 "#   {scheme designator: <list of keywords for current cid>\n",
                 "#    scheme_designator: ...}\n",
                 "\n",
+                "from typing import Dict, List\n",
+                "\n\n",
             ]
         )
-        f.write("name_for_cid = {}\n")
-        f.write("cid_concepts = {}\n")
+        f.write("name_for_cid: Dict[int, str] = {}\n")
+        f.write("cid_concepts: Dict[int, Dict[str, List[str]]] = {}\n")
         for cid, value in cid_lists.items():
             # cid: int
             # value: Dict[str, List[str]]
@@ -273,12 +284,14 @@ def write_concept_files(concepts: ConceptType) -> None:
         f"# Auto-generated by pydicom-data-sr\n",
         "# -*- coding: utf-8 -*-\n",
         "\n",
+        "from typing import Dict, List, Tuple\n",
+        "\n\n",
     ]
 
     imports = []
     top_indent = " " * 4
     m_indent = " " * 8
-    for scheme, top_value in concepts.items():
+    for scheme, attr_codes in concepts.items():
         module = f"_concepts_{scheme}"
         variable = f"concepts_{scheme}"
         imports.append((scheme, module, variable))
@@ -292,21 +305,21 @@ def write_concept_files(concepts: ConceptType) -> None:
             # concepts_scheme: {
             #     <top_value>,
             # }
-            f.write(f"{variable} = {{\n")
-            for name, middle_value in sorted(top_value.items(), key=lambda x: x[0]):
-                # name: str
-                # middle_value: Dict[str, Tuple[str, List[int]]]
+            f.write(f"{variable}: Dict[str, Dict[str, Tuple[str, List[int]]]] = {{\n")
+            for attr, codes in sorted(attr_codes.items(), key=lambda x: x[0]):
+                # attr: str
+                # codes: Dict[str, Tuple[str, List[int]]]
                 # Write as:
-                #     name: {
+                #     cc_name: {
                 #         <middle_value>,
                 #     },
-                f.write(f'{top_indent}"{name}": {{\n')
-                for key, val in sorted(middle_value.items(), key=lambda x: x[0]):
-                    # key: str
-                    # val: Tuple[str, List[int]]
+                f.write(f'{top_indent}"{attr}": {{\n')
+                for code, (meaning, cids) in sorted(codes.items(), key=lambda x: x[0]):
+                    # code: str
+                    # (meaning, cids): Tuple[str, List[int]]
                     # Write as:
-                    #         key: (str, List[int]),
-                    f.write(f'{m_indent}"{key}": ("{val[0]}", {sorted(val[1])}),\n')
+                    #         code: (str, List[int]),
+                    f.write(f'{m_indent}"{code}": ("{meaning}", {sorted(cids)}),\n')
 
                 f.write(f"{top_indent}}},\n")
 
@@ -317,11 +330,13 @@ def write_concept_files(concepts: ConceptType) -> None:
     with open(CONCEPTS_FILE, "w", encoding="utf8") as f:
         f.writelines(header)
         for _, module, variable in imports:
-            # from sr.tables._cid_concepts_<scheme> import _concepts_<scheme>
+            # from sr.tables._concepts_<scheme> import concepts_<scheme>
             f.write(f"from sr.tables.{module} import {variable}\n")
 
         f.write("\n\n")
-        f.write("concepts = {\n")
+        f.write(
+            "concepts: Dict[str, Dict[str, Dict[str, Tuple[str, List[int]]]]] = {\n"
+        )
         for scheme, _, variable in imports:
             f.write(f'    "{scheme}": {variable},\n')
         f.write("}\n")
@@ -331,9 +346,6 @@ def write_version_file(dicom_version: str) -> None:
     """Write a new _version.py file"""
 
     new_version = datetime.now().strftime("%Y.%m.%d")
-    if new_version == __version__:
-        raise RuntimeError("Error updating the package: no change in version number")
-
     with open(VERSION_FILE, "w") as f:
         f.write(f'__version__: str = "{new_version}"\n')
         f.write(f'__dicom_version__: str = "{dicom_version}"\n')
@@ -348,27 +360,15 @@ def _setup_argparser() -> Any:
 
     # General Options
     gen_opts = parser.add_argument_group("General Options")
-    gen_opts.add_argument(
-        "-d",
-        "--dev",
-        help="enable dev mode",
-        action="store_true",
-    )
     gen_opts.add_argument(
         "--force-download",
-        help="force downloading the data tables",
+        help="force downloading the data tables to a local directory",
         action="store_true",
         default=False,
     )
     gen_opts.add_argument(
         "--force-regeneration",
-        help="force regenerating the data tables",
-        action="store_true",
-        default=False,
-    )
-    gen_opts.add_argument(
-        "--clean",
-        help="remove all data files",
+        help="force regenerating the data tables from local source data",
         action="store_true",
         default=False,
     )
@@ -381,11 +381,14 @@ def _setup_argparser() -> Any:
     logging.basicConfig(level=logging.DEBUG)
     args = _setup_argparser()
 
-    src = None
-    if args.dev:
-        src = PACKAGE_DIR / "temp"
+    LOCAL_DIR = PACKAGE_DIR / "temp"
 
-    if args.clean:
-        pass
+    src = None
+    if args.force_regeneration or args.force_download:
+        src = LOCAL_DIR
 
-    run(src, args.force_download, args.force_regeneration)
+    result = run(src, args.force_download, args.force_regeneration)
+    if "GITHUB_ACTION" in os.environ:
+        subprocess.run(
+            f"echo 'PACKAGE_UPDATED={str(result)}' >> $GITHUB_ENV", shell=True
+        )