Merge pull request #411 from linkml/schemaview_local_remote_import_tests

Fix the remote import issue in SchemaViewer and a couple of misc minor bugs

Author: sierra-moxon

linkml_runtime/utils/schemaview.py

@@ -141,6 +141,9 @@ def is_absolute_path(path: str) -> bool:
     """
     if path.startswith("/"):
         return True
+    if "://" in path:
+        return True
+
     # windows
     if not os.path.isabs(path):
         return False
@@ -445,39 +448,20 @@ def ordered(self, elements: ElementDict, ordered_by: OrderedBy | None = None) ->
     def _order_lexically(self, elements: ElementDict) -> ElementDict:
         """Order elements by name.
 
+        Note: sort is case sensitive, based on the name used in the original schema.
+
         :param element: slots or class type to order
         :return: all classes or slots sorted lexically in schema view
         """
-        ordered_list_of_names = []
-        ordered_elements = {}
-        for c in elements:
-            ordered_list_of_names.append(c)
-        ordered_list_of_names.sort()
-        for name in ordered_list_of_names:
-            ordered_elements[self.get_element(name).name] = self.get_element(name)
-        return ordered_elements
+        return {name: elements[name] for name in sorted(elements.keys())}
 
     def _order_rank(self, elements: ElementDict) -> ElementDict:
         """Order elements by rank.
 
         :param elements: slots or classes to order
         :return: all classes or slots sorted by their rank in schema view
         """
-        rank_map = {}
-        unranked_map = {}
-        rank_ordered_elements = {}
-        for name, definition in elements.items():
-            if definition.rank is None:
-                unranked_map[self.get_element(name).name] = self.get_element(name)
-
-            else:
-                rank_map[definition.rank] = name
-        rank_ordered_map = collections.OrderedDict(sorted(rank_map.items()))
-        for k, v in rank_ordered_map.items():
-            rank_ordered_elements[self.get_element(v).name] = self.get_element(v)
-
-        rank_ordered_elements.update(unranked_map)
-        return rank_ordered_elements
+        return {el.name: el for el in sorted(elements.values(), key=lambda el: el.rank or float("inf"))}
 
     def _order_inheritance(self, elements: DefDict) -> DefDict:
         """Sort classes such that if C is a child of P then C appears after P in the list."""
@@ -697,7 +681,7 @@ def get_class(self, class_name: CLASS_NAME, imports: bool = True, strict: bool =
         """
         c = self.all_classes(imports=imports).get(class_name, None)
         if strict and c is None:
-            raise ValueError(f'No such class as "{class_name}"')
+            raise ValueError(f'No such class: "{class_name}"')
         return c
 
     @lru_cache(None)
@@ -723,7 +707,7 @@ def get_slot(
                     slot.from_schema = c.from_schema
                     slot.owner = c.name
         if strict and slot is None:
-            raise ValueError(f'No such slot as "{slot_name}"')
+            raise ValueError(f'No such slot: "{slot_name}"')
         return slot
 
     @lru_cache(None)
@@ -736,7 +720,7 @@ def get_subset(self, subset_name: SUBSET_NAME, imports: bool = True, strict: boo
         """
         s = self.all_subsets(imports).get(subset_name, None)
         if strict and s is None:
-            raise ValueError(f'No such subset as "{subset_name}"')
+            raise ValueError(f'No such subset: "{subset_name}"')
         return s
 
     @lru_cache(None)
@@ -749,7 +733,7 @@ def get_enum(self, enum_name: ENUM_NAME, imports: bool = True, strict: bool = Fa
         """
         e = self.all_enums(imports).get(enum_name, None)
         if strict and e is None:
-            raise ValueError(f'No such subset as "{enum_name}"')
+            raise ValueError(f'No such enum: "{enum_name}"')
         return e
 
     @lru_cache(None)
@@ -762,7 +746,7 @@ def get_type(self, type_name: TYPE_NAME, imports: bool = True, strict: bool = Fa
         """
         t = self.all_types(imports).get(type_name, None)
         if strict and t is None:
-            raise ValueError(f'No such subset as "{type_name}"')
+            raise ValueError(f'No such type: "{type_name}"')
         return t
 
     def _parents(self, e: Element, imports: bool = True, mixins: bool = True, is_a: bool = True) -> list[ElementName]:

tests/test_utils/test_schemaview.py

@@ -38,6 +38,10 @@
 SCHEMA_RELATIVE_IMPORT_TREE = Path(INPUT_DIR) / "imports_relative" / "L0_0" / "L1_0_0" / "main.yaml"
 SCHEMA_RELATIVE_IMPORT_TREE2 = Path(INPUT_DIR) / "imports_relative" / "L0_2" / "main.yaml"
 
+CREATURE_SCHEMA = "creature_schema"
+CREATURE_SCHEMA_BASE_URL = "https://github.com/linkml/linkml-runtime/tests/test_utils/input/mcc"
+CREATURE_SCHEMA_BASE_PATH = Path(INPUT_DIR) / "mcc"
+
 yaml_loader = YAMLLoader()
 IS_CURRENT = "is current"
 EMPLOYED_AT = "employed at"
@@ -81,6 +85,23 @@ def sv_attributes() -> SchemaView:
     return SchemaView(os.path.join(INPUT_DIR, "attribute_edge_cases.yaml"))
 
 
+@pytest.fixture(scope="session")
+def creature_view() -> SchemaView:
+    return SchemaView(str(CREATURE_SCHEMA_BASE_PATH / "creature_schema.yaml"))
+
+
+@pytest.fixture(scope="session")
+def creature_view_remote() -> SchemaView:
+    """Fixture for a SchemaView for testing remote imports."""
+    return SchemaView(str(CREATURE_SCHEMA_BASE_PATH / "creature_schema_remote.yaml"))
+
+
+@pytest.fixture(scope="session")
+def creature_view_local() -> SchemaView:
+    """Fixture for a SchemaView for testing local relative file path imports."""
+    return SchemaView(str(CREATURE_SCHEMA_BASE_PATH / "creature_schema_local.yaml"))
+
+
 def make_schema(
     name: str,
     prefixes: list[Prefix] | None = None,
@@ -307,6 +328,35 @@ def sv_induced_slots() -> SchemaView:
     return SchemaView(schema_str)
 
 
+@pytest.fixture
+def sv_ordering_tests() -> SchemaView:
+    """SchemaView for testing class ordering."""
+    schema = """
+id: https://example.com/ordering-tests
+name: ordering-tests
+classes:
+    Clarinet:
+        rank: 5
+        is_a: wind instrument
+    instrument:
+        rank: 2
+    Bassoon:
+        is_a: wind instrument
+    wind instrument:
+        rank: 1
+        is_a: instrument
+    Abacus:
+        is_a: counting instrument
+    counting instrument:
+        rank: 4
+        is_a: instrument
+    Didgeridoo:
+        rank: 3
+        is_a: wind instrument
+"""
+    return SchemaView(schema)
+
+
 def test_imports(schema_view_with_imports: SchemaView) -> None:
     """View should by default dynamically include imports chain."""
     view = schema_view_with_imports
@@ -697,6 +747,145 @@ def test_uris_without_default_prefix() -> None:
     assert view.get_uri("test_slot", imports=True) == "https://example.org/test#test_slot"
 
 
+CREATURE_EXPECTED = {
+    "class": {
+        CREATURE_SCHEMA: {"MythicalCreature", "HasMagic", "MagicalAbility", "Dragon", "Phoenix", "Unicorn"},
+        "creature_basics": {"Entity", "Creature", "Location", "CreatureAttribute", "HasHabitat"},
+    },
+    "slot": {
+        CREATURE_SCHEMA: {"creature_class", "magical_abilities", "level_of_magic"},
+        "creature_basics": {"id", "name", "description", "species", "habitat"},
+    },
+    "type": {
+        CREATURE_SCHEMA: {"MagicLevel"},
+        "creature_types": {"string", "integer", "boolean"},
+    },
+    "enum": {CREATURE_SCHEMA: {"CreatureClass"}},
+    "subset": {CREATURE_SCHEMA: set(), "creature_subsets": {"mythical_creature", "generic_creature"}},
+    "element": {},
+}
+
+# add in the elements
+for value in CREATURE_EXPECTED.values():
+    for s_name, el in value.items():
+        if s_name not in CREATURE_EXPECTED["element"]:
+            CREATURE_EXPECTED["element"][s_name] = set()
+        CREATURE_EXPECTED["element"][s_name].update(el)
+
+
+@pytest.mark.parametrize("schema", ["creature_view", "creature_view_remote", "creature_view_local"])
+@pytest.mark.parametrize("entity", CREATURE_EXPECTED.keys())
+def test_creature_schema_entities_with_without_imports(
+    schema: str, entity: str, request: pytest.FixtureRequest
+) -> None:
+    """Test retrieval of entities from the creature schema.
+
+    Tests the following methods:
+    - all_{entity}s (e.g. all_classes, all_slots, etc.) with and without imports
+    - get_{entity} (e.g. get_class, get_slot, etc.)
+    - the "from_schema" attribute of the retrieved entities
+
+    The schemas tested are:
+    - creature_view: the main schema with all entities
+    - creature_view_remote: imports the creature schema using a curie and a remote URL
+    - creature_view_local: imports the creature schema using a local relative file path
+    """
+    creature_view = request.getfixturevalue(schema)
+
+    # use the PLURAL mapping to get the correct method name to retrieve all entities of the given type
+    get_all_fn = "all_" + PLURAL.get(entity, f"{entity}s")
+    if schema == "creature_view":
+        assert set(getattr(creature_view, get_all_fn)(imports=False)) == CREATURE_EXPECTED[entity][CREATURE_SCHEMA]
+    else:
+        assert set(getattr(creature_view, get_all_fn)(imports=False)) == set()
+
+    # merge the values from all the sources to get the complete set of entities
+    all_entities = set().union(*CREATURE_EXPECTED[entity].values())
+    assert set(getattr(creature_view, get_all_fn)(imports=True)) == all_entities
+
+    # run creature_view.get_{entity} for each entity and check the source
+    for src in CREATURE_EXPECTED[entity]:
+        for entity_name in CREATURE_EXPECTED[entity][src]:
+            e = getattr(creature_view, f"get_{entity}")(entity_name, imports=True)
+            assert e.from_schema == f"{CREATURE_SCHEMA_BASE_URL}/{src}"
+
+
+@pytest.mark.parametrize("entity", CREATURE_EXPECTED.keys())
+def test_get_entities_with_without_imports(creature_view: SchemaView, entity: str) -> None:
+    """Test retrieval of a specific entity from the creature schema."""
+    get_fn = f"get_{entity}"
+
+    for src in CREATURE_EXPECTED[entity]:
+        for entity_name in CREATURE_EXPECTED[entity][src]:
+            if src == CREATURE_SCHEMA:
+                # if the source is the main schema, we can use the method directly
+                e = getattr(creature_view, get_fn)(entity_name, imports=False)
+                assert e.name == entity_name
+                # N.b. BUG: due to caching and how the `from_schema` element is generated,
+                # we cannot know whether it will be populated.
+                # assert e.from_schema is None
+            else:
+                # if the source is an imported schema, we expect None without imports
+                assert getattr(creature_view, get_fn)(entity_name, imports=False) is None
+                if entity != "element":
+                    # in strict mode, we expect an error if the entity does not exist
+                    with pytest.raises(ValueError, match=f'No such {entity}: "{entity_name}"'):
+                        getattr(creature_view, f"get_{entity}")(entity_name, imports=False, strict=True)
+
+            # turn on imports
+            e = getattr(creature_view, f"get_{entity}")(entity_name, imports=True)
+            assert e.from_schema == f"{CREATURE_SCHEMA_BASE_URL}/{src}"
+
+
+@pytest.mark.parametrize("entity", argvalues=[e for e in CREATURE_EXPECTED if e != "element"])
+def test_get_entity_does_not_exist(creature_view: SchemaView, entity: str) -> None:
+    """Test retrieval of a specific entity from the creature schema."""
+    get_fn = f"get_{entity}"
+
+    # returns None unless the `strict` flag is passed
+    assert getattr(creature_view, get_fn)("does_not_exist") is None
+
+    # raises an error with `strict` flag on
+    with pytest.raises(ValueError, match=f'No such {entity}: "does_not_exist"'):
+        getattr(creature_view, get_fn)("does_not_exist", strict=True)
+
+
+ORDERING_TESTS = {
+    # Bassoon and Abacus are unranked, so appear at the end of the list.
+    "rank": ["wind instrument", "instrument", "Didgeridoo", "counting instrument", "Clarinet", "Bassoon", "Abacus"],
+    "preserve": [
+        "Clarinet",
+        "instrument",
+        "Bassoon",
+        "wind instrument",
+        "Abacus",
+        "counting instrument",
+        "Didgeridoo",
+    ],
+    # lexical ordering is case-sensitive, so all the capitalized words come first.
+    "lexical": ["Abacus", "Bassoon", "Clarinet", "Didgeridoo", "counting instrument", "instrument", "wind instrument"],
+    # TODO: this looks very dodgy
+    "inheritance": [
+        "instrument",
+        "wind instrument",
+        "Clarinet",
+        "Bassoon",
+        "counting instrument",
+        "Abacus",
+        "Didgeridoo",
+    ],
+}
+
+
+@pytest.mark.parametrize(
+    ("ordered_by"),
+    ORDERING_TESTS.keys(),
+)
+def test_all_classes_ordered_by(sv_ordering_tests: SchemaView, ordered_by: str) -> None:
+    """Test the ordered_by method."""
+    assert list(sv_ordering_tests.all_classes(ordered_by=ordered_by).keys()) == ORDERING_TESTS[ordered_by]
+
+
 def test_children_method(schema_view_no_imports: SchemaView) -> None:
     """Test retrieval of the children of a class."""
     view = schema_view_no_imports