issue(medcat-den): CU-869c96xwg Improve model card adaption (#352)

* CU-869c96xwg: Add v1 model pack to resources

* CU-869c96xwg: Use/unpack v1 test model in base package for tests

* CU-869c96xwg: Add a test for getting model card directly from unpacked v1 model

* CU-869c96xwg: Fix issue with legacy / half-filled model card

* CU-869c96xwg: Force-unpack v1 model within test (instead of globally)

* CU-869c96xwg: Improve model card tests with copies of dicts

* CU-869c96xwg: Add a couple of simple tests for model card load / validation data integrity

Author: mart-r

medcat-den/src/medcat_den/base.py

@@ -35,10 +35,28 @@ def make_permissive(cls, v: dict) -> dict:
             defaults = {
                 'Pipeline Description': {"core": {}, "addons": []},
                 'Required Plugins': [],
+                "Location": "N/A",
+                "Basic CDB Stats": {
+                    "Unsupervised training history": [],
+                    "Supervised training history": [],
+                },
+                "Source Ontology": ["unknown"],
             }
             out_dict = {**defaults, **v}  # v overwrites defaults
-            if out_dict.get("Source Ontology") is None:
-                out_dict['Source Ontology'] = ['Unknown']
+            cls._check_key_value_recursively(out_dict, defaults)
             return out_dict
         return v
 
+    @classmethod
+    def _check_key_value_recursively(
+            cls, out_dict: dict, defaults: dict) -> None:
+        for key, def_val in defaults.items():
+            # NOTE: this should be mostly for nested stuff
+            if key not in out_dict:
+                out_dict[key] = def_val
+                continue
+            cur_val = out_dict[key]
+            if cur_val is None or type(cur_val) is not type(def_val):
+                out_dict[key] = def_val
+            elif isinstance(def_val, dict):
+                cls._check_key_value_recursively(cur_val, def_val)

medcat-den/tests/__init__.py

@@ -7,15 +7,21 @@
 
 MODEL_PATH = os.path.join(
     os.path.dirname(__file__), "resources", "mct2_model_pack.zip")
+V1_MODEL_PATH = os.path.join(
+    os.path.dirname(MODEL_PATH), "mct_v1_model_pack.zip"
+)
 
 
 # unpack
 _model_folder = CAT.attempt_unpack(MODEL_PATH)
+_v1_model_folder = CAT.attempt_unpack(V1_MODEL_PATH)
 
 
 def remove_model_folder():
     if os.path.exists(_model_folder):
         shutil.rmtree(_model_folder)
+    if os.path.exists(_v1_model_folder):
+        shutil.rmtree(_v1_model_folder)
 
 
 # cleanup

medcat-den/tests/resources/mct_v1_model_pack.zip

@@ -1,3 +1,5 @@
+from copy import deepcopy
+
 from medcat_den.base import ModelInfo
 
 
@@ -40,8 +42,8 @@
 def test_validates_with_old_format():
     model = ModelInfo(
         model_id="test_id",
-        model_card=MODEL_CARD_NO_NEW_KEYS,
-            base_model=None,
+        model_card=deepcopy(MODEL_CARD_NO_NEW_KEYS),
+        base_model=None,
         model_name="test_model",
         model_num=1,
     )
@@ -51,8 +53,8 @@ def test_validates_with_old_format():
 def test_validates_with_new_format():
     model = ModelInfo(
         model_id="test_id",
-        model_card=MODEL_CARD_WITH_NEW_KEYS,
-            base_model=None,
+        model_card=deepcopy(MODEL_CARD_WITH_NEW_KEYS),
+        base_model=None,
         model_name="test_model",
         model_num=1,
     )
@@ -62,11 +64,41 @@ def test_validates_with_new_format():
 def test_new_format_keeps_values():
     model = ModelInfo(
         model_id="test_id",
-        model_card=MODEL_CARD_WITH_NEW_KEYS,
-            base_model=None,
+        model_card=deepcopy(MODEL_CARD_WITH_NEW_KEYS),
+        base_model=None,
         model_name="test_model",
         model_num=1,
     )
     mc = model.model_card
     for key, exp_value in NEW_KV.items():
         assert exp_value == mc[key]
+
+
+def test_new_format_does_not_change_dict():
+    model = ModelInfo(
+        model_id="test_id",
+        model_card=deepcopy(MODEL_CARD_WITH_NEW_KEYS),
+        base_model=None,
+        model_name="test_model",
+        model_num=1,
+    )
+    assert model.model_card == MODEL_CARD_WITH_NEW_KEYS
+
+
+def test_old_format_will_not_change_twice():
+    model_with_fixed_mc = ModelInfo(
+        model_id="test_id",
+        model_card=deepcopy(MODEL_CARD_NO_NEW_KEYS),
+        base_model=None,
+        model_name="test_model",
+        model_num=1,
+    )
+    model = ModelInfo(
+        model_id="test_id",
+        model_card=model_with_fixed_mc.model_card,
+        base_model=None,
+        model_name="test_model",
+        model_num=1,
+    )
+    assert model.model_card == model_with_fixed_mc.model_card
+    assert model.model_card != MODEL_CARD_NO_NEW_KEYS

medcat-den/tests/test_model_card_validity.py

@@ -0,0 +1,27 @@
+import os
+import json
+
+from medcat_den import base
+
+
+from . import V1_MODEL_PATH
+
+import pytest
+
+
+@pytest.fixture
+def v1_model_card():
+    model_card_path = os.path.join(
+        V1_MODEL_PATH.removesuffix(".zip"), "model_card.json")
+    # NOTE: for some reason, this doesn't exist at this point
+    from medcat.cat import CAT
+    CAT.attempt_unpack(V1_MODEL_PATH)
+    with open(model_card_path) as f:
+        return json.load(f)
+
+
+def test_can_get_model_card(v1_model_card):
+    model_info = base.ModelInfo(model_id=v1_model_card["Model ID"],
+                                model_card=v1_model_card,
+                                base_model=None)
+    assert isinstance(model_info, base.ModelInfo)