diff --git a/linkml_runtime/utils/schemaview.py b/linkml_runtime/utils/schemaview.py index 1511ca06..db479121 100644 --- a/linkml_runtime/utils/schemaview.py +++ b/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, diff --git a/tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_0/child.yaml b/tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_0/child.yaml new file mode 100644 index 00000000..c3e714f5 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_1/child.yaml b/tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_1/child.yaml new file mode 100644 index 00000000..574e41c8 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/L1_0_0/main.yaml b/tests/test_utils/input/imports_relative/L0_0/L1_0_0/main.yaml new file mode 100644 index 00000000..ad5c48a2 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/L1_0_0/neighbor.yaml b/tests/test_utils/input/imports_relative/L0_0/L1_0_0/neighbor.yaml new file mode 100644 index 00000000..f5635ef2 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/L1_0_1/L2_0_1_0/grandchild.yaml b/tests/test_utils/input/imports_relative/L0_0/L1_0_1/L2_0_1_0/grandchild.yaml new file mode 100644 index 00000000..6a712980 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/L1_0_1/dupe.yaml b/tests/test_utils/input/imports_relative/L0_0/L1_0_1/dupe.yaml new file mode 100644 index 00000000..2223f898 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/neighborhood_parent.yaml b/tests/test_utils/input/imports_relative/L0_0/neighborhood_parent.yaml new file mode 100644 index 00000000..1f94d4bc --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_0/parent.yaml b/tests/test_utils/input/imports_relative/L0_0/parent.yaml new file mode 100644 index 00000000..f482e1d7 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_0/apple.yaml b/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_0/apple.yaml new file mode 100644 index 00000000..742e27bd --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_0/index.yaml b/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_0/index.yaml new file mode 100644 index 00000000..685f5e61 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_1/banana.yaml b/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_1/banana.yaml new file mode 100644 index 00000000..88dd3bb8 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_1/index.yaml b/tests/test_utils/input/imports_relative/L0_1/L1_1_0/L2_1_0_1/index.yaml new file mode 100644 index 00000000..fae27ce5 --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_1/L1_1_0/index.yaml b/tests/test_utils/input/imports_relative/L0_1/L1_1_0/index.yaml new file mode 100644 index 00000000..84efda1f --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/L0_1/cousin.yaml b/tests/test_utils/input/imports_relative/L0_1/cousin.yaml new file mode 100644 index 00000000..0f82a56a --- /dev/null +++ b/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" \ No newline at end of file diff --git a/tests/test_utils/input/imports_relative/README.md b/tests/test_utils/input/imports_relative/README.md new file mode 100644 index 00000000..a9f97ec7 --- /dev/null +++ b/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. \ No newline at end of file diff --git a/tests/test_utils/test_schemaview.py b/tests/test_utils/test_schemaview.py index 367fb842..de2634d6 100644 --- a/tests/test_utils/test_schemaview.py +++ b/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): """