linkml / linkml-runtime

Runtime support for linkml generated models
https://linkml.io/linkml/
Creative Commons Zero v1.0 Universal
24 stars 23 forks source link

Schemaview: follow paths in multi-layer relative imports #330

Closed sneakers-the-rat closed 3 weeks ago

sneakers-the-rat commented 1 month ago

Fix: https://github.com/linkml/linkml/issues/1228 Specifically: https://github.com/linkml/linkml/issues/1228#issuecomment-1712390959

1228 describes two problems, one having to do with relative imports when using an already-loaded SchemaDefinition object with SchemaView, which has already been fixed, and another, where multiple layers of relative imports wouldn't work.

eg. Say you have three schema such that...

Then a_schema would be correctly imported, but b_schema wouldn't be, since it would previously be resolved relative to the directory of main.yaml

Additionally, one might anticipate schema developers to use an index.yaml style of creating nested schemas, where there might potentially be many schemas with a filename index.yaml in different directories - we need to preserve those same-named schemas while also not getting into import cycles.

This simple lil PR does that, and tests a bunch of different permutations of nth-layer imports (see tests/test_utils/input/imports_relative/README.md )

This has to be done in the imports_closure method (rather than in the schema_wrap method, or making imports_closure pass the loaded schema as the from_schema argument) in order to preserve same-named schemas in the import_map, which uses the string of the import (rather than eg. schema.id or schema.name) as the key. Note that while this does mean that the key in import_map will be different than the literal import string in the nth-layer schema (eg. in the above example, the key would be ./a/b/b_schema.yaml while the actual import statement was ./b/b_schema.yaml ), that string is not meaningful to the origin schema, and the relative import from its location is unambiguous and allows for deduplication (which is tested)

yet another "two lines of code but 1000 lines of tests" PR lol

sneakers-the-rat commented 1 month ago

ping on this :) would like to remove my monkeypatch <3

jgadling commented 3 weeks ago

This is blocking some functionality I'm trying to implement with LinkML as well. Is there an ETA on merge/release?

sierra-moxon commented 3 weeks ago

it looks like this is the error blocking atm:

Because linkml-dataops (0.1.0) depends on linkml-runtime (>=1.1.6)
 and no versions of linkml-dataops match !=0.1.0, every version of linkml-dataops requires linkml-runtime (>=1.1.6).
So, because linkml depends on both linkml-dataops (*) and linkml-runtime (0.0.0) @ file:///home/runner/work/linkml-runtime/linkml-runtime/linkml-runtime, version solving failed.
Error: Process completed with exit code 1.
sneakers-the-rat commented 3 weeks ago

That was a bug with the upstream test runner that has since been fixed - trigger a manual run of that upstream test action and it should work ;)

sneakers-the-rat commented 3 weeks ago

tried to trigger a run, but i don't have privileges to run it manually for this repo, and it only runs when there is an approval event, which is proving to be increasingly annoying and i'm thinking we should just always run the upstream test action lol