Merge pull request #350 from ialarmedalien/bugfix/issue-2422

Ensure that relative imports can be imported without requiring `./` in front of the import file name

Author: sierra-moxon

README.md

@@ -11,7 +11,7 @@ Runtime support for linkml generated data classes.
 
 ## About
 
-This python library provides runtime support for [LinkML](https://linkml.io/linkml/) datamodels.
+This Python library provides runtime support for [LinkML](https://linkml.io/linkml/) datamodels.
 
 See the [LinkML repo](https://github.com/linkml/linkml) for the [Python Dataclass Generator](https://linkml.io/linkml/generators/python.html) which will convert a schema into a Python object model. That model will have dependencies on functionality in this library.
 
@@ -24,8 +24,8 @@ See [working with data](https://linkml.io/linkml/data/index.html) in the documen
 
 This repository also contains the Python dataclass representation of the [LinkML metamodel](https://github.com/linkml/linkml-model), and various utility functions that are useful for working with LinkML data and schemas.
 
-It also includes the [SchemaView](https://linkml.io/linkml/developers/manipulating-schemas.html) class for working with LinkML schemas
+It also includes the [SchemaView](https://linkml.io/linkml/developers/manipulating-schemas.html) class for working with LinkML schemas.
 
 ## Notebooks
 
-See the [notebooks](https://github.com/linkml/linkml-runtime/tree/main/notebooks) folder for examples
+See the [notebooks](https://github.com/linkml/linkml-runtime/tree/main/notebooks) folder for examples.

linkml_runtime/utils/schemaview.py

@@ -108,25 +108,6 @@ def is_absolute_path(path: str) -> bool:
     drive, tail = os.path.splitdrive(norm_path)
     return bool(drive and tail)
 
-def _resolve_import(source_sch: str, imported_sch: str) -> str:
-    if os.path.isabs(imported_sch):
-        # Absolute import paths are not modified
-        return imported_sch
-    if urlparse(imported_sch).scheme:
-        # File with URL schemes are not modified
-        return imported_sch
-    
-    if WINDOWS:
-        path = PurePath(os.path.normpath(PurePath(source_sch).parent / imported_sch)).as_posix()
-    else:
-        path = os.path.normpath(str(Path(source_sch).parent / imported_sch))
-
-    if imported_sch.startswith(".") and not path.startswith("."):
-        # Above condition handles cases where both source schema and imported schema are relative paths: these should remain relative
-        return f"./{path}"
-
-    return path
-
 
 @dataclass
 class SchemaUsage:
@@ -319,14 +300,24 @@ def imports_closure(self, imports: bool = True, traverse: Optional[bool] = None,
                     # 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
+                    # essential to sensible deduplication: e.g. 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.
-                    i = _resolve_import(sn, i)
+
+                    # if i is not a CURIE and sn looks like a path with at least one parent folder,
+                    # normalise i with respect to sn
+                    if "/" in sn and ":" not in i:
+                        if WINDOWS:
+                            # This cannot be simplified. os.path.normpath() must be called before .as_posix()
+                            i = PurePath(
+                                os.path.normpath(PurePath(sn).parent / i)
+                            ).as_posix()
+                        else:
+                            i = os.path.normpath(str(Path(sn).parent / i))
                     todo.append(i)
 
             # add item to closure

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_2/four.yaml

@@ -0,0 +1,13 @@
+id: four
+name: import_four
+title: Import Four
+description: |
+  Import loaded by the StepChild class.
+imports:
+  - linkml:types
+classes:
+  Four:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Four"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_2/one.yaml

@@ -0,0 +1,14 @@
+id: one
+name: import_one
+title: Import One
+description: |
+  Import loaded by the StepChild class.
+imports:
+  - linkml:types
+  - two
+classes:
+  One:
+    attributes:
+      value:
+        range: string
+        ifabsent: "One"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_2/stepchild.yaml

@@ -0,0 +1,16 @@
+id: stepchild
+name: stepchild
+title: stepchild
+description: |
+  Child class that imports files in the same directory as itself without consistently using `./` in the link notation.
+imports:
+  - linkml:types
+  - one
+  - two
+  - ./three
+classes:
+  StepChild:
+    attributes:
+      value:
+        range: string
+        ifabsent: "StepChild"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_2/three.yaml

@@ -0,0 +1,14 @@
+id: three
+name: import_three
+title: Import Three
+description: |
+  Import loaded by the StepChild class.
+imports:
+  - linkml:types
+  - ./four
+classes:
+  Three:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Three"

tests/test_utils/input/imports_relative/L0_0/L1_0_0/L2_0_0_2/two.yaml

@@ -0,0 +1,13 @@
+id: two
+name: import_two
+title: Import Two
+description: |
+  Import loaded by the StepChild class.
+imports:
+  - linkml:types
+classes:
+  Two:
+    attributes:
+      value:
+        range: string
+        ifabsent: "Two"

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

@@ -8,10 +8,11 @@ imports:
   - ../../L0_1/cousin
   - ./L2_0_0_0/child
   - ./L2_0_0_1/child
+  - L2_0_0_2/stepchild
 classes:
   Main:
     description: "Our intrepid main class!"
     attributes:
       value:
         range: string
-        ifabsent: "Main"
+        ifabsent: "Main"

tests/test_utils/test_schemaview.py

@@ -19,7 +19,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_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'
 SCHEMA_RELATIVE_IMPORT_TREE2 = Path(INPUT_DIR) / 'imports_relative' / 'L0_2' / 'main.yaml'
@@ -357,7 +357,7 @@ def test_caching():
     view.add_class(ClassDefinition('X'))
     assert len(['X']) == len(view.all_classes())
     view.add_class(ClassDefinition('Y'))
-    assert len(['X', 'Y']) ==  len(view.all_classes())
+    assert len(['X', 'Y']) == len(view.all_classes())
     # bypass view method and add directly to schema;
     # in general this is not recommended as the cache will
     # not be updated
@@ -546,6 +546,11 @@ def test_imports_relative():
         '../L1_0_1/dupe',
         './L2_0_0_0/child',
         './L2_0_0_1/child',
+        'L2_0_0_2/two',
+        'L2_0_0_2/one',
+        'L2_0_0_2/four',
+        'L2_0_0_2/three',
+        'L2_0_0_2/stepchild',
         'main'
     ]
 
@@ -716,6 +721,7 @@ def test_slot_inheritance():
     with pytest.raises(ValueError):
         view.slot_ancestors('s5')
 
+
 def test_attribute_inheritance():
     """
     Tests attribute inheritance edge cases.