diff --git a/linkml_runtime/utils/schemaview.py b/linkml_runtime/utils/schemaview.py index e792366a..b53ea5cf 100644 --- a/linkml_runtime/utils/schemaview.py +++ b/linkml_runtime/utils/schemaview.py @@ -4,9 +4,10 @@ import collections from functools import lru_cache from copy import copy, deepcopy -from collections import defaultdict +from collections import defaultdict, deque from pathlib import Path from typing import Mapping, Tuple +import warnings from linkml_runtime.utils.namespaces import Namespaces from deprecated.classic import deprecated @@ -207,34 +208,76 @@ def load_import(self, imp: str, from_schema: SchemaDefinition = None): return schema @lru_cache() - def imports_closure(self, imports: bool = True, traverse=True, inject_metadata=True) -> List[SchemaDefinitionName]: + def imports_closure(self, imports: bool = True, traverse: Optional[bool] = None, inject_metadata=True) -> List[SchemaDefinitionName]: """ Return all imports - :param traverse: if true, traverse recursively + Objects in imported classes override one another in a "python-like" order - + from the point of view of the importing schema, imports will override one + another from first to last, recursively for each layer of imports. + + An import tree like:: + + - main + - s1 + - s1_1 + - s1_2 + - s1_2_1 + - s1_2_2 + - s2 + - s2_1 + - s2_2 + + will override objects with the same name, in order:: + + ['s1_1', 's1_2_1', 's1_2_2', 's1_2', 's1', 's2_1', 's2_2', 's2'] + + :param imports: bool (default: ``True`` ) include imported schemas, recursively + :param traverse: bool, optional (default: ``True`` ) (Deprecated, use + ``imports`` ). if true, traverse recursively :return: all schema names in the transitive reflexive imports closure """ - if not imports: - return [self.schema.name] if self.schema_map is None: self.schema_map = {self.schema.name: self.schema} - closure = [] + + closure = deque() visited = set() todo = [self.schema.name] - if not traverse: + + if traverse is not None: + warnings.warn( + 'traverse behaves identically to imports and will be removed in a future version. Use imports instead.', + DeprecationWarning + ) + + if not imports or (not traverse and traverse is not None): return todo + while len(todo) > 0: + # visit item sn = todo.pop() - visited.add(sn) if sn not in self.schema_map: imported_schema = self.load_import(sn) self.schema_map[sn] = imported_schema - s = self.schema_map[sn] - if sn not in closure: - closure.append(sn) - for i in s.imports: - if i not in visited: - todo.append(i) + + # resolve item's imports if it has not been visited already + # we will get duplicates, but not cycles this way, and + # filter out dupes, preserving the first entry, at the end. + if sn not in visited: + for i in self.schema_map[sn].imports: + # no self imports ;) + if i != sn: + todo.append(i) + + # add item to closure + # append + pop (above) is FILO queue, which correctly extends tree leaves, + # but in backwards order. + closure.appendleft(sn) + visited.add(sn) + + # filter duplicates, keeping first entry + closure = list({k:None for k in closure}.keys()) + if inject_metadata: for s in self.schema_map.values(): for x in {**s.classes, **s.enums, **s.slots, **s.subsets, **s.types}.values(): @@ -434,6 +477,7 @@ def all_elements(self, imports=True) -> Dict[ElementName, Element]: def _get_dict(self, slot_name: str, imports=True) -> Dict: schemas = self.all_schema(imports) d = {} + # pdb.set_trace() # iterate through all schemas and merge the list together for s in schemas: # get the value of element name from the schema, if empty, return empty dictionary. diff --git a/tests/test_issues/test_linkml_issue_998.py b/tests/test_issues/test_linkml_issue_998.py index 191e0729..b847c762 100644 --- a/tests/test_issues/test_linkml_issue_998.py +++ b/tests/test_issues/test_linkml_issue_998.py @@ -81,8 +81,8 @@ def test_slots_are_not_duplicated(view): def test_issue_998_attribute_slot(view): enum_slots = view.get_slots_by_enum("EmploymentStatusEnum") - assert len(enum_slots) == 1 - assert enum_slots[0].name == "employed" + assert len(enum_slots) == 2 + assert sorted([slot.name for slot in enum_slots]) == ["employed", "past_employer"] def test_issue_998_schema_and_attribute_slots(view): diff --git a/tests/test_utils/input/imports/README.md b/tests/test_utils/input/imports/README.md new file mode 100644 index 00000000..fc991304 --- /dev/null +++ b/tests/test_utils/input/imports/README.md @@ -0,0 +1,33 @@ +A tree of imports like: + +``` +main +|- linkml:types +|- s1 +| |- s1_1 +| |- s1_2 +| |- s1_2_1 +| |- s1_2_1_1 +| |- s1_2_1_1_1 +| |- s1_2_1_1_2 +|- s2 +| |- s2_1 +| |- s2_2 +|- s3 + |- s3_1 + |- s3_2 +``` + +This is used to test SchemaView's logic for complex import hierarchies, +eg. +- overrides +- imports closure completeness +- (at some point) monotonicity +- etc. + +Currently, each schema... +- Contains one `Main` class with a value whose default is overridden to indicate which module defined it, this is used to test overrides. +- Contains a class like `S2` that carries the name of the module to ensure that unique classes are gotten from the whole tree +- Each node in the tree outwards from `main` will carry the 'special classes' from the parents, overriding them as with `main` to + get a more detailed picture of override order: eg. `s1_2_1` will also define `S1_2` and override its `ifabsent` field, + which `S1_2` should override since it's the importing schema. \ No newline at end of file diff --git a/tests/test_utils/input/imports/main.yaml b/tests/test_utils/input/imports/main.yaml new file mode 100644 index 00000000..40b77593 --- /dev/null +++ b/tests/test_utils/input/imports/main.yaml @@ -0,0 +1,15 @@ +id: main +name: main +title: main +imports: + - linkml:types + - s1 + - s2 + - s3 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "Main" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1.yaml b/tests/test_utils/input/imports/s1.yaml new file mode 100644 index 00000000..83ac7bfe --- /dev/null +++ b/tests/test_utils/input/imports/s1.yaml @@ -0,0 +1,20 @@ +id: s1 +name: s1 +title: s1 +imports: + - linkml:types + - s1_1 + - s1_2 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1_1.yaml b/tests/test_utils/input/imports/s1_1.yaml new file mode 100644 index 00000000..56944a3b --- /dev/null +++ b/tests/test_utils/input/imports/s1_1.yaml @@ -0,0 +1,24 @@ +id: s1_1 +name: s1_1 +title: s1_1 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1_1" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_1" + S1_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1_2.yaml b/tests/test_utils/input/imports/s1_2.yaml new file mode 100644 index 00000000..837999bb --- /dev/null +++ b/tests/test_utils/input/imports/s1_2.yaml @@ -0,0 +1,25 @@ +id: s1_2 +name: s1_2 +title: s1_2 +imports: + - linkml:types + - s1_2_1 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1_2" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2" + S1_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1_2_1.yaml b/tests/test_utils/input/imports/s1_2_1.yaml new file mode 100644 index 00000000..ba30e9b6 --- /dev/null +++ b/tests/test_utils/input/imports/s1_2_1.yaml @@ -0,0 +1,31 @@ +id: s1_2_1 +name: s1_2_1 +title: s1_2_1 +imports: + - linkml:types + - s1_2_1_1 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1_2_1" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1" + S1_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1" + S1_2_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1_2_1_1.yaml b/tests/test_utils/input/imports/s1_2_1_1.yaml new file mode 100644 index 00000000..f306d46e --- /dev/null +++ b/tests/test_utils/input/imports/s1_2_1_1.yaml @@ -0,0 +1,38 @@ +id: s1_2_1_1 +name: s1_2_1_1 +title: s1_2_1_1 +imports: + - linkml:types + - s1_2_1_1_1 + - s1_2_1_1_2 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1" + S1_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1" + S1_2_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1" + S1_2_1_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1_2_1_1_1.yaml b/tests/test_utils/input/imports/s1_2_1_1_1.yaml new file mode 100644 index 00000000..8ac9032c --- /dev/null +++ b/tests/test_utils/input/imports/s1_2_1_1_1.yaml @@ -0,0 +1,42 @@ +id: s1_2_1_1_1 +name: s1_2_1_1_1 +title: s1_2_1_1_1 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_1" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_1" + S1_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_1" + S1_2_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_1" + S1_2_1_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_1" + S1_2_1_1_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s1_2_1_1_2.yaml b/tests/test_utils/input/imports/s1_2_1_1_2.yaml new file mode 100644 index 00000000..67393ce9 --- /dev/null +++ b/tests/test_utils/input/imports/s1_2_1_1_2.yaml @@ -0,0 +1,42 @@ +id: s1_2_1_1_2 +name: s1_2_1_1_2 +title: s1_2_1_1_2 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_2" + S1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_2" + S1_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_2" + S1_2_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_2" + S1_2_1_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_2" + S1_2_1_1_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S1_2_1_1_2" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s2.yaml b/tests/test_utils/input/imports/s2.yaml new file mode 100644 index 00000000..09abda0f --- /dev/null +++ b/tests/test_utils/input/imports/s2.yaml @@ -0,0 +1,20 @@ +id: s2 +name: s2 +title: s2 +imports: + - linkml:types + - s2_1 + - s2_2 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S2" + S2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S2" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s2_1.yaml b/tests/test_utils/input/imports/s2_1.yaml new file mode 100644 index 00000000..69dbfa46 --- /dev/null +++ b/tests/test_utils/input/imports/s2_1.yaml @@ -0,0 +1,24 @@ +id: s2_1 +name: s2_1 +title: s2_1 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S2_1" + S2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S2_1" + S2_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S2_1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s2_2.yaml b/tests/test_utils/input/imports/s2_2.yaml new file mode 100644 index 00000000..eb26f137 --- /dev/null +++ b/tests/test_utils/input/imports/s2_2.yaml @@ -0,0 +1,24 @@ +id: s2_2 +name: s2_2 +title: s2_2 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S2_2" + S2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S2_2" + S2_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S2_2" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s3.yaml b/tests/test_utils/input/imports/s3.yaml new file mode 100644 index 00000000..dc373be8 --- /dev/null +++ b/tests/test_utils/input/imports/s3.yaml @@ -0,0 +1,20 @@ +id: s3 +name: s3 +title: s3 +imports: + - linkml:types + - s3_1 + - s3_2 +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S3" + S3: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S3" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s3_1.yaml b/tests/test_utils/input/imports/s3_1.yaml new file mode 100644 index 00000000..01441816 --- /dev/null +++ b/tests/test_utils/input/imports/s3_1.yaml @@ -0,0 +1,24 @@ +id: s3_1 +name: s3_1 +title: s3_1 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S3_1" + S3: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S3_1" + S3_1: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S3_1" \ No newline at end of file diff --git a/tests/test_utils/input/imports/s3_2.yaml b/tests/test_utils/input/imports/s3_2.yaml new file mode 100644 index 00000000..06f1ab08 --- /dev/null +++ b/tests/test_utils/input/imports/s3_2.yaml @@ -0,0 +1,24 @@ +id: s3_2 +name: s3_2 +title: s3_2 +imports: + - linkml:types +classes: + Main: + description: "The class we use to test overrides on imports!" + attributes: + value: + range: string + ifabsent: "S3_2" + S3: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S3_2" + S3_2: + description: "A class from one of the imported classes!" + attributes: + value: + range: string + ifabsent: "S3_2" \ No newline at end of file diff --git a/tests/test_utils/test_schemaview.py b/tests/test_utils/test_schemaview.py index 53ea6f01..825b3cf1 100644 --- a/tests/test_utils/test_schemaview.py +++ b/tests/test_utils/test_schemaview.py @@ -18,6 +18,7 @@ SCHEMA_NO_IMPORTS = Path(INPUT_DIR) / 'kitchen_sink_noimports.yaml' 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' yaml_loader = YAMLLoader() IS_CURRENT = 'is current' @@ -478,6 +479,50 @@ def test_imports_from_schemaview(self): self.assertCountEqual(view.all_classes(), view2.all_classes()) self.assertCountEqual(view.all_classes(imports=False), view2.all_classes(imports=False)) + def test_imports_closure_order(self): + """ + Imports should override in a python-like order. + + See + - https://github.com/linkml/linkml/issues/1839 for initial discussion + - input/imports/README.md for explanation of the test schema + """ + sv = SchemaView(SCHEMA_IMPORT_TREE) + closure = sv.imports_closure(imports=True) + target = [ + 'linkml:types', + 's1_1', + 's1_2_1_1_1', 's1_2_1_1_2', + 's1_2_1_1', 's1_2_1', 's1_2', + 's1', + 's2_1', 's2_2', 's2', + 's3_1', 's3_2', 's3', + 'main' + ] + self.assertEqual(closure, target) + + def test_imports_overrides(self): + """ + Classes defined in the importing module should override same-named classes in + imported modules. + + Tests recursively across an import tree. Each class defines all classes lower + in the tree with a `value` attribute with an `ifabsent` value matching the + current schema. Lower (closer to the importing schema) schemas should override + each class at that level or lower, keeping the rest. + + See `input/imports/README.md` for further explanation. + """ + sv = SchemaView(SCHEMA_IMPORT_TREE) + defaults = {} + target = {} + for name, cls in sv.all_classes(imports=True).items(): + target[name] = name + defaults[name] = cls.attributes['value'].ifabsent + + self.assertEqual(defaults, target) + + def test_direct_remote_imports(self): """ Tests that building a SchemaView directly from a remote URL works.