Merge pull request #467 from linkml/schemaview_pv_fn_edits

schemaview.py: make `permissible_value_*` act consistently with other such methods

Author: kevinschaper

linkml_runtime/utils/schemaview.py

@@ -820,41 +820,37 @@ def enum_parents(
     @lru_cache(None)
     def permissible_value_parent(
         self, permissible_value: str, enum_name: ENUM_NAME
-    ) -> list[str | PermissibleValueText] | None:
+    ) -> list[str | PermissibleValueText]:
         """:param enum_name: child enum name
         :param permissible_value: permissible value
         :return: all direct parent enum names (is_a)
         """
         enum = self.get_enum(enum_name, strict=True)
-        if enum:
-            if permissible_value in enum.permissible_values:
-                pv = enum.permissible_values[permissible_value]
-                if pv.is_a:
-                    return [pv.is_a]
-            return None
-        return []
+        pv = enum.permissible_values.get(permissible_value)
+        if not pv:
+            err_msg = f'"{permissible_value}" is not a permissible value of the enum "{enum_name}".'
+            raise ValueError(err_msg)
+        return [pv.is_a] if pv.is_a else []
 
     @lru_cache(None)
     def permissible_value_children(
         self, permissible_value: str, enum_name: ENUM_NAME
-    ) -> list[str | PermissibleValueText] | None:
+    ) -> list[str | PermissibleValueText]:
         """:param enum_name: parent enum name
         :param permissible_value: permissible value
         :return: all direct child permissible values (is_a)
         """
         enum = self.get_enum(enum_name, strict=True)
-        children = []
-        if enum:
-            if permissible_value in enum.permissible_values:
-                pv = enum.permissible_values[permissible_value]
-                for isapv in enum.permissible_values:
-                    isapv_entity = enum.permissible_values[isapv]
-                    if isapv_entity.is_a and pv.text == isapv_entity.is_a:
-                        children.append(isapv)
-                return children
-            return None
-        msg = f'No such enum as "{enum_name}"'
-        raise ValueError(msg)
+        pv = enum.permissible_values.get(permissible_value)
+        if not pv:
+            err_msg = f'"{permissible_value}" is not a permissible value of the enum "{enum_name}".'
+            raise ValueError(err_msg)
+
+        return [
+            isa_pv
+            for isa_pv, isa_pv_entity in enum.permissible_values.items()
+            if isa_pv_entity.is_a and pv.text == isa_pv_entity.is_a
+        ]
 
     @lru_cache(None)
     def slot_parents(

tests/test_utils/test_schemaview.py

@@ -1106,8 +1106,12 @@ def test_alias_slot(schema_view_no_imports: SchemaView) -> None:
     assert postal_code_slot.alias == "zip"  # Assert alias is 'zip'
 
 
-def test_enums_and_enum_relationships(schema_view_no_imports: SchemaView) -> None:
-    """Test various aspects of Enum representation.
+def test_permissible_value_relationships(schema_view_no_imports: SchemaView) -> None:
+    """Test relationships between permissible values.
+
+    These tests use valid permissible_value / enum combinations.
+
+    See test_permissible_value_relationships_fail for invalid enum-PV pairings.
 
     CAT:
     LION:
@@ -1122,18 +1126,14 @@ def test_enums_and_enum_relationships(schema_view_no_imports: SchemaView) -> Non
     """
     view = schema_view_no_imports
 
-    # Test for ValueError when passing incorrect parameters
-    with pytest.raises(ValueError, match='No such enum: "not_an_enum"'):
-        view.permissible_value_parent("not_a_pv", "not_an_enum")
-
     animals = "Animals"
     animal_enum = view.get_enum(animals)
     assert animal_enum.name == animals
 
     pv_cat = animal_enum.permissible_values["CAT"]
     assert pv_cat.text == "CAT"
     assert pv_cat.is_a is None
-    assert view.permissible_value_parent("CAT", animals) is None
+    assert view.permissible_value_parent("CAT", animals) == []
     assert view.permissible_value_ancestors("CAT", animals) == ["CAT"]
     assert set(view.permissible_value_children("CAT", animals)) == {"LION", "TABBY"}
     assert set(view.permissible_value_descendants("CAT", animals)) == {"CAT", "LION", "ANGRY_LION", "TABBY"}
@@ -1160,6 +1160,19 @@ def test_enums_and_enum_relationships(schema_view_no_imports: SchemaView) -> Non
     assert view.permissible_value_descendants("ANGRY_LION", animals) == ["ANGRY_LION"]
 
 
+@pytest.mark.parametrize("fn", ["parent", "children", "ancestors", "descendants"])
+def test_permissible_value_relationships_fail(schema_view_no_imports: SchemaView, fn: str) -> None:
+    """Test permissible_value relationships with incorrect enum/PV pairs."""
+    method_name = f"permissible_value_{fn}"
+    # invalid enum
+    with pytest.raises(ValueError, match='No such enum: "invalid_enum"'):
+        getattr(schema_view_no_imports, method_name)("invalid_pv", "invalid_enum")
+
+    # invalid pv, valid enum
+    with pytest.raises(ValueError, match='"invalid_pv" is not a permissible value of the enum "Animals"'):
+        getattr(schema_view_no_imports, method_name)("invalid_pv", "Animals")
+
+
 # FIXME: improve testing of dynamic enums
 def test_dynamic_enum(schema_view_with_imports: SchemaView) -> None:
     """Rudimentary test of dynamic enum."""