Schemaview: follow paths in multi-layer relative imports

linkml_runtime/utils/schemaview.py

@@ -279,8 +279,25 @@ def imports_closure(self, imports: bool = True, traverse: Optional[bool] = None,
             if sn not in visited:
                 for i in self.schema_map[sn].imports:
                     # no self imports ;)
-                    if i != sn:
-                        todo.append(i)
+                    if i == sn:
+                        continue
+
+                    # resolve relative imports relative to the importing schema, rather than the
+                    # origin schema. Imports can be a URI or Curie, and imports from the same
+                    # directory don't require a ./, so if the current (sn) import is a relative
+                    # path, and the target import doesn't have : (as in a curie or a URI)
+                    # we prepend the relative path. This WILL make the key in the `schema_map` not
+                    # equal to the literal text specified in the importing schema, but this is
+                    # essential to sensible deduplication: eg. for
+                    # - main.yaml (imports ./types.yaml, ./subdir/subschema.yaml)
+                    # - types.yaml
+                    # - subdir/subschema.yaml (imports ./types.yaml)
+                    # - subdir/types.yaml
+                    # we should treat the two `types.yaml` as separate schemas from the POV of the
+                    # origin schema.
+                    if sn.startswith('.') and ':' not in i:
+                        i = os.path.normpath(str(Path(sn).parent / i))
+                    todo.append(i)
 
             # add item to closure
             # append + pop (above) is FILO queue, which correctly extends tree leaves,

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_0/child.yaml

@@ -0,0 +1,15 @@
+id: child
+name: child
+title: child
+description: |
+  Child class that shares the same name and ID as another child and should *not* be deduplicated, 
+  but imports the dupe schema which *should* be deduplicated
+imports:
+  - linkml:types
+  - ../../L1_0_1/dupe
+classes:
+  Child1:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Child1"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_1/child.yaml

@@ -0,0 +1,15 @@
+id: child
+name: child
+title: child
+description: |
+  Child class that shares the same name and ID as another child and should *not* be deduplicated, 
+  but imports the dupe schema which *should* be deduplicated
+imports:
+  - linkml:types
+  - ../../L1_0_1/dupe
+classes:
+  Child2:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Child2"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/main.yaml

@@ -0,0 +1,17 @@
+id: main
+name: main
+title: main
+imports:
+  - linkml:types
+  - neighbor
+  - ../parent
+  - ../../L0_1/cousin
+  - ./L2_0_0_0/child
+  - ./L2_0_0_1/child
+classes:
+  Main:
+    description: "Our intrepid main class!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "Main"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/neighbor.yaml

@@ -0,0 +1,13 @@
+id: neighbor
+name: neighbor
+title: neighbor
+description: neighbor class imported without ./, but has relative imports of its own
+imports:
+  - ../neighborhood_parent
+classes:
+  Neighbor:
+    description: "Our main class's best friend!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "Neighbor"

tests/test_utils/input/imports_relative/L0_0/L1_0_1/L2_0_1_0/grandchild.yaml

@@ -0,0 +1,13 @@
+id: grandchild
+name: grandchild
+title: grandchild
+description: Grandchild schema that should cause a cycle with parent if we are just naively concatenating paths
+imports:
+  - ../../parent
+classes:
+  Grandchild:
+    description: "spoiled rotten!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "Grandchild"

tests/test_utils/input/imports_relative/L0_0/L1_0_1/dupe.yaml

@@ -0,0 +1,11 @@
+id: dupe
+name: dupe
+title: dupe
+description: A Duplicate schema that is imported from multiple places (but should only actually be imported once)
+classes:
+  Dupe:
+    description: "A class from the duplicated import!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "Dupe"

tests/test_utils/input/imports_relative/L0_0/neighborhood_parent.yaml

@@ -0,0 +1,11 @@
+id: neighborhood_parent
+name: neighborhood_parent
+title: neighborhood_parent
+description: parent of same-directory import
+classes:
+  Neighborhood_Parent:
+    description: "Keeps the cul-de-sac fed and scolds the speeding traffic"
+    attributes:
+      value:
+        range: string
+        ifabsent: "Neighborhood_Parent"

tests/test_utils/input/imports_relative/L0_0/parent.yaml

@@ -0,0 +1,13 @@
+id: parent
+name: parent
+title: parent
+description: Parent of our main schema, imports two layers down in the grandchild
+imports:
+  - linkml:types
+  - ./L1_0_1/L2_0_1_0/grandchild
+classes:
+  Parent:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Parent"

tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_0/apple.yaml

@@ -0,0 +1,9 @@
+id: apple
+name: apple
+title: apple
+classes:
+  Apple:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Apple"

tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_0/index.yaml

@@ -0,0 +1,12 @@
+id: L1_1_0/L2_1_0_0/index
+name: index
+title: index
+imports:
+  - apple
+classes:
+  L2100Index:
+    description: "A class from an index!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "L2100Index"

tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_1/banana.yaml

@@ -0,0 +1,9 @@
+id: banana
+name: banana
+title: banana
+classes:
+  Banana:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Banana"

tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_1/index.yaml

@@ -0,0 +1,12 @@
+id: L1_1_0/L2_1_0_1/index
+name: index
+title: index
+imports:
+  - ./banana
+classes:
+  L2101Index:
+    description: "A class from an index!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "L2101Index"

tests/test_utils/input/imports_relative/L0_1/L1_1_0/index.yaml

@@ -0,0 +1,13 @@
+id: L1_1_0/index
+name: index
+title: index
+imports:
+  - ./L2_1_0_0/index
+  - ./L2_1_0_1/index
+classes:
+  L110Index:
+    description: "A class from an index!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "L110Index"

tests/test_utils/input/imports_relative/L0_1/cousin.yaml

@@ -0,0 +1,14 @@
+id: cousin
+name: cousin
+title: cousin
+description: Testing for "index" style imports where the same name is used multiple times in a non-duplicating way
+imports:
+  - linkml:types
+  - ./L1_1_0/index
+classes:
+  Cousin:
+    description: "The cousin of our intrepid main class!"
+    attributes:
+      value:
+        range: string
+        ifabsent: "Cousin"

tests/test_utils/input/imports_relative/README.md

@@ -0,0 +1,65 @@
+# Relative Imports test schemas
+
+This tests the ability for schemaview to follow chains of relative imports - ie. that it resolves 
+each schema's relative imports with respect to the *imported* schema rather than the *origin* schema.
+
+This test case handles resolving relative imports in both directions, as well as neighboring directories
+starting from `L0_0/L1_0_0/main.yaml` schema. Each directory is labeled with a "level" (`L{n}`) and a second number for 
+the "column" of the directory - eg `L1_1_*` is within `L0_1`, and each subdirectory adds more children.
+
+Note that this does **not** test that overrides are parsed correctly, that is tested in the `imports` directory
+next to this - this is specifically about relative file handling (the same import ordering should happen regardless of
+where the imports are located)
+
+The schema in this directory make a graph like this, starting from main (as absolute paths)
+
+```
+main                            --> linkml:types
+main                            --> L0_0/L1_0_0/neighbor
+main                            --> L0_0/parent
+main                            --> L0_1/cousin
+main                            --> L0_0/L1_0_0/L2_0_0_0/child
+main                            --> L0_0/L1_0_0/L2_0_0_1/child
+L0_0/L1_0_0/neighbor            --> L0_0/neighborhood_parent
+L0_0/parent                     --> L0_0/L1_0_1/L2_0_1_0/grandchild
+L0_0/L1_0_1/L2_0_1_0/grandchild --> L0_0/parent
+L0_1/cousin                     --> L0_1/L1_1_0/index
+L0_1/L1_1_0/index               --> L0_1/L1_1_0/L2_1_0_0/index
+L0_1/L1_1_0/index               --> L0_1/L1_1_0/L2_1_0_1/index
+L0_1/L1_1_0/L2_1_0_0/index      --> L0_1/L1_1_0/L2_1_0_0/apple
+L0_1/L1_1_0/L2_1_0_1/index      --> L0_1/L1_1_0/L2_1_0_1/banana
+L0_0/L1_0_0/L2_0_0_0/child      --> L0_0/L1_0_1/dupe 
+L0_0/L1_0_0/L2_0_0_1/child      --> L0_0/L1_0_1/dupe 
+
+```
+
+From the perspective of the main schema, we should end up with a resolved set of imports like:
+
+```
+- linkml:types'
+- ../neighborhood_parent
+- neighbor
+- ../parent
+- ../L1_0_1/L2_0_1_0/grandchild
+- ../../L0_1/L1_1_0/L2_1_0_0/apple
+- ../../L0_1/L1_1_0/L2_1_0_0/index
+- ../../L0_1/L1_1_0/L2_1_0_1/banana
+- ../../L0_1/L1_1_0/L2_1_0_1/index
+- ../../L0_1/L1_1_0/index
+- ../../L0_1/cousin
+- ../L1_0_1/dupe
+- ./L2_0_0_0/child
+- ./L2_0_0_1/child
+- main
+```
+
+(see the `imports` test directory and `test_imports_closure_order` for notes on ordering)
+
+The specific things tested by the schemas are:
+- same-directory import
+- child directory import (`./`)
+- parent directory import (`../`)
+- double child and parent skips (`../../`, `./path/path`)
+- schemas with duplicate names (`child.yaml`, `index.yaml`) that should all be imported
+- multiple imports of the same schema (`dupe.yaml`)
+- cycles: since importing will mutate the path, ensure that we don't end up importing the same schemas forever.

tests/test_utils/test_schemaview.py

@@ -19,6 +19,7 @@
 SCHEMA_WITH_IMPORTS = Path(INPUT_DIR) / 'kitchen_sink.yaml'
 SCHEMA_WITH_STRUCTURED_PATTERNS = Path(INPUT_DIR) / "pattern-example.yaml"
 SCHEMA_IMPORT_TREE = Path(INPUT_DIR) / 'imports' / 'main.yaml'
+SCHEMA_RELATIVE_IMPORT_TREE = Path(INPUT_DIR) / 'imports_relative' / 'L0_0' / 'L1_0_0' / 'main.yaml'
 
 yaml_loader = YAMLLoader()
 IS_CURRENT = 'is current'
@@ -577,6 +578,41 @@ def test_imports_overrides(self):
 
         self.assertEqual(defaults, target)
 
+    def test_imports_relative(self):
+        """
+        Relative imports from relative imports should evaluate relative to the *importing* schema,
+        not the *origin* schema.
+
+        See
+            - input/imports_relative/README.md for an explanation of the test schema
+        """
+        sv = SchemaView(SCHEMA_RELATIVE_IMPORT_TREE)
+        closure = sv.imports_closure(imports=True)
+
+        assert len(closure) == len(sv.schema_map.keys())
+        assert closure == [
+            'linkml:types',
+            '../neighborhood_parent',
+            'neighbor',
+            '../parent',
+            '../L1_0_1/L2_0_1_0/grandchild',
+            '../../L0_1/L1_1_0/L2_1_0_0/apple',
+            '../../L0_1/L1_1_0/L2_1_0_0/index',
+            '../../L0_1/L1_1_0/L2_1_0_1/banana',
+            '../../L0_1/L1_1_0/L2_1_0_1/index',
+            '../../L0_1/L1_1_0/index',
+            '../../L0_1/cousin',
+            '../L1_0_1/dupe',
+            './L2_0_0_0/child',
+            './L2_0_0_1/child',
+            'main'
+        ]
+
+        # check that we can actually get the classes from the same-named schema
+        classes = sv.all_classes(imports=True)
+        assert 'L110Index' in classes
+        assert 'L2100Index' in classes
+        assert 'L2101Index' in classes
 
     def test_direct_remote_imports(self):
         """