Skip to content

Fix detection of CURIES vs. absolute paths on Windows #391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 19, 2025
Merged

Fix detection of CURIES vs. absolute paths on Windows #391

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7a5102
Fix detection of CURIES vs. absolute paths on Windows
dalito May 15, 2025
1a53d82
Fix type checking
dalito May 15, 2025
02523d9
Add test for context_utils.map_import & switch to pytest
dalito May 15, 2025
fa6d966
Fix test to work cross platform
dalito May 15, 2025
cd17c76
Install all extras for upstream linkml tests (fix pandera tests)
dalito May 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 5 additions & 10 deletions .github/workflows/test-upstream.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
run: |
set +e
set +o pipefail

UPSTREAM_BRANCH=$( \
echo "$PR_BODY" | \
head -2 | \
Expand All @@ -52,12 +52,12 @@ jobs:
)
echo "Got upstream branch:"
echo $UPSTREAM_BRANCH

if [[ -z "$UPSTREAM_BRANCH" ]]; then
echo "Using main as default"
UPSTREAM_BRANCH="main"
fi

UPSTREAM_REPO=$( \
echo "$PR_BODY" | \
head -2 | \
Expand All @@ -67,7 +67,7 @@ jobs:
)
echo "Got upstream repo:"
echo $UPSTREAM_REPO

if [[ -z "$UPSTREAM_REPO" ]]; then
echo "Using linkml/linkml as default"
UPSTREAM_REPO="linkml/linkml"
Expand Down Expand Up @@ -128,11 +128,6 @@ jobs:
path: linkml/.venv
key: venv-${{ matrix.python-version }}-${{ runner.os }}-${{ hashFiles('**/poetry.lock') }}

# make extra sure we're removing any old version of linkml-runtime that exists
- name: uninstall potentially cached linkml-runtime
working-directory: linkml
run: poetry run pip uninstall -y linkml-runtime

# we are not using linkml-runtime's lockfile, but simulating what will happen
# when we merge this and update linkml's lockfile
- name: add linkml-runtime to lockfile
Expand All @@ -145,7 +140,7 @@ jobs:
# the cache will still speedup the rest of the installation
- name: install linkml
working-directory: linkml
run: poetry install --no-interaction -E tests
run: poetry sync --no-interaction --all-extras

- name: print linkml-runtime version
working-directory: linkml
Expand Down
13 changes: 10 additions & 3 deletions linkml_runtime/utils/context_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
import yaml
from jsonasobj2 import JsonObj, loads

from typing import TYPE_CHECKING

if TYPE_CHECKING:
from linkml_runtime.utils.namespaces import Namespaces

Check warning on line 12 in linkml_runtime/utils/context_utils.py

View check run for this annotation

Codecov / codecov/patch

linkml_runtime/utils/context_utils.py#L12 

Added line #L12 was not covered by tests


CONTEXT_TYPE = Union[str, dict, JsonObj]
CONTEXTS_PARAM_TYPE = Optional[Union[CONTEXT_TYPE, list[CONTEXT_TYPE]]]

Expand Down Expand Up @@ -50,7 +56,7 @@
JsonObj(**{"@context": context_list[0] if len(context_list) == 1 else context_list})


def map_import(importmap: dict[str, str], namespaces: Callable[[None], "Namespaces"], imp: Any) -> str:
def map_import(importmap: dict[str, str], namespaces: Callable[[], "Namespaces"], imp: Any) -> str:
"""
lookup an import in an importmap.

Expand All @@ -73,7 +79,8 @@
else:
sname = os.path.join(expanded_prefix, lname)
sname = importmap.get(sname, sname) # Import map may use CURIE
sname = str(namespaces().uri_for(sname)) if ':' in sname else sname
if ':' in sname and not ":\\" in sname: # Don't interpret Windows paths as CURIEs
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know there's at least one other place (apart from the one below) where there's a similar check to try to determine if something is a CURIE or not. I wonder if it'd be better to extract this out into a separate function -- I know it's an extremely simple check, but maybe it would lessen the chance of similar errors occurring elsewhere. There's a bug in the SchemaViewer that stops remote schemas being imported from other remote schemas, and it's due to this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if it is worth to move the check into a function. I still hope that one day we will use more specific types (Path, URI, CURIE) instead of string. This would make the code much cleaner.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, that's the best way to fix the issue. One day...!

sname = str(namespaces().uri_for(sname))
return importmap.get(sname, sname) # It may also use URI or other forms


Expand Down Expand Up @@ -103,7 +110,7 @@
if base:
outmap = dict()
for k, v in rval.items():
if ':' not in v:
if ':' not in v or ":\\" in v: # Don't interpret Windows paths as CURIEs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

v = os.path.join(os.path.abspath(base), v)
outmap[k] = v
rval = outmap
Expand Down
110 changes: 70 additions & 40 deletions tests/test_utils/test_context_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
import unittest
import os
import pytest

from jsonasobj2 import JsonObj, loads
from rdflib import URIRef

from linkml_runtime.linkml_model.meta import ClassDefinition, Prefix, SchemaDefinition
from linkml_runtime.utils.context_utils import map_import, merge_contexts
from linkml_runtime.utils.namespaces import Namespaces

from linkml_runtime.utils.context_utils import merge_contexts
from tests.test_utils import METAMODEL_CONTEXT_URI, META_BASE_URI

json_1 = '{ "ex": "http://example.org/test/", "ex2": "http://example.org/test2/" }'
Expand All @@ -26,43 +31,47 @@
}"""


class ContextUtilsTestCase(unittest.TestCase):
def test_merge_contexts(self):
self.assertIsNone(merge_contexts())
self.assertEqual('file://local.jsonld', merge_contexts("local.jsonld")['@context'])
self.assertEqual('file://local.jsonld', merge_contexts(["local.jsonld"])['@context'])
self.assertEqual(METAMODEL_CONTEXT_URI, merge_contexts(METAMODEL_CONTEXT_URI)['@context'])
self.assertEqual(METAMODEL_CONTEXT_URI, merge_contexts([METAMODEL_CONTEXT_URI])['@context'])
self.assertEqual(JsonObj(ex='http://example.org/test/', ex2='http://example.org/test2/'),
merge_contexts(json_1)['@context'])
self.assertEqual(JsonObj(ex='http://example.org/test/', ex2='http://example.org/test2/'),
merge_contexts([json_1])['@context'])
self.assertEqual(JsonObj(ex='http://example.org/test3/', ex2=JsonObj(**{'@id': 'http://example.org/test4/'})),
merge_contexts(json_2)['@context'])
self.assertEqual(JsonObj(ex='http://example.org/test3/', ex2=JsonObj(**{'@id': 'http://example.org/test4/'})),
merge_contexts([json_2])['@context'])
self.assertEqual([f'file://local.jsonld',
'https://w3id.org/linkml/meta.context.jsonld',
JsonObj(ex='http://example.org/test/', ex2='http://example.org/test2/'),
JsonObj(ex='http://example.org/test3/', ex2=JsonObj(**{'@id': 'http://example.org/test4/'}))],
merge_contexts(["local.jsonld", METAMODEL_CONTEXT_URI, json_1, json_2])['@context'])
self.assertEqual(loads(context_output),
merge_contexts(["local.jsonld", METAMODEL_CONTEXT_URI, json_1, json_2]))
def test_merge_contexts():
assert merge_contexts() is None
assert "file://local.jsonld" == merge_contexts("local.jsonld")["@context"]
assert "file://local.jsonld" == merge_contexts(["local.jsonld"])["@context"]
assert METAMODEL_CONTEXT_URI == merge_contexts(METAMODEL_CONTEXT_URI)["@context"]
assert METAMODEL_CONTEXT_URI == merge_contexts([METAMODEL_CONTEXT_URI])["@context"]
assert JsonObj(ex="http://example.org/test/", ex2="http://example.org/test2/") == merge_contexts(json_1)["@context"]
assert (
JsonObj(ex="http://example.org/test/", ex2="http://example.org/test2/") == merge_contexts([json_1])["@context"]
)
assert (
JsonObj(ex="http://example.org/test3/", ex2=JsonObj(**{"@id": "http://example.org/test4/"}))
== merge_contexts(json_2)["@context"]
)
assert (
JsonObj(ex="http://example.org/test3/", ex2=JsonObj(**{"@id": "http://example.org/test4/"}))
== merge_contexts([json_2])["@context"]
)
assert [
f"file://local.jsonld",
"https://w3id.org/linkml/meta.context.jsonld",
JsonObj(ex="http://example.org/test/", ex2="http://example.org/test2/"),
JsonObj(ex="http://example.org/test3/", ex2=JsonObj(**{"@id": "http://example.org/test4/"})),
] == merge_contexts(["local.jsonld", METAMODEL_CONTEXT_URI, json_1, json_2])["@context"]
assert loads(context_output) == merge_contexts(["local.jsonld", METAMODEL_CONTEXT_URI, json_1, json_2])
# Dups are not removed
assert JsonObj(
**{
"@context": [
JsonObj(ex="http://example.org/test/", ex2="http://example.org/test2/"),
JsonObj(ex="http://example.org/test/", ex2="http://example.org/test2/"),
]
}
) == merge_contexts([json_1, json_1])
assert "file://local.jsonld", merge_contexts("local.jsonld")["@context"]

# Dups are not removed
self.assertEqual(
JsonObj(**{'@context': [JsonObj(ex='http://example.org/test/', ex2='http://example.org/test2/'),
JsonObj(ex='http://example.org/test/', ex2='http://example.org/test2/')]}),
merge_contexts([json_1, json_1]))
self.assertEqual('file://local.jsonld', merge_contexts("local.jsonld")['@context'])

def test_merge_contexts_base(self):
self.assertEqual(
JsonObj(**{'@context':
JsonObj(**{'@base': 'file://relloc'})}),
merge_contexts(base='file://relloc'))
self.assertEqual(loads(f'{{"@context": {{"@base": "{META_BASE_URI}"}}}}'), merge_contexts(base=META_BASE_URI))
self.assertEqual(loads("""
def test_merge_contexts_base():
assert JsonObj(**{"@context": JsonObj(**{"@base": "file://relloc"})}) == merge_contexts(base="file://relloc")
assert loads(f'{{"@context": {{"@base": "{META_BASE_URI}"}}}}') == merge_contexts(base=META_BASE_URI)
assert loads("""
{"@context": [
"https://w3id.org/linkml/meta.context.jsonld",
{
Expand All @@ -79,8 +88,29 @@ def test_merge_contexts_base(self):
"@base": "https://w3id.org/linkml/"
}
]
}"""), merge_contexts([METAMODEL_CONTEXT_URI, json_1, json_2], base=META_BASE_URI))
}""") == merge_contexts([METAMODEL_CONTEXT_URI, json_1, json_2], base=META_BASE_URI)


@pytest.mark.parametrize(
("imp", "result"),
[
("linkml:types", f"C:\\temp\\linkml_model\\model\\schema{os.sep}types"),
("ex_file", "/tmp/example/schema"),
("_types", "https://w3id.org/linkml/types"),
],
)
def test_map_import(imp, result):
importmap = {
"linkml:": "C:\\temp\\linkml_model\\model\\schema",
"ex_file": "/tmp/example/schema",
"_types": "linkml:types",
}

def namespaces():
ns = Namespaces()
ns["linkml"] = URIRef("https://w3id.org/linkml/")
ns["ex_file"] = URIRef("https://example.org/file/")
ns["_types"] = URIRef("https://w3id.org/linkml/types/")
return ns

if __name__ == '__main__':
unittest.main()
assert map_import(importmap, namespaces, imp) == result
Loading