Skip to content
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.

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

linkml-files gives incorrect URIs for JSON-LD contexts #2047

Open
sneakers-the-rat opened this issue Apr 3, 2024 · 2 comments · May be fixed by linkml/linkml-runtime#320
Open

linkml-files gives incorrect URIs for JSON-LD contexts #2047

sneakers-the-rat opened this issue Apr 3, 2024 · 2 comments · May be fixed by linkml/linkml-runtime#320
Labels
bug Something that should work but isn't, with an example and a test case. linkml-runtime

Comments

@sneakers-the-rat
Copy link
Collaborator

Describe the bug
I am afraid to say this was my doing, and it may be cold comfort when i say that i'm doing more to both fix this and prevent it from happening again. I'm sorry. there is a good ending here though.

linkml_runtime.linkml_model.linkml_files recently got a revamp: linkml/linkml-runtime#310

That module is the source for a lot of top-level constants in linkml - notable for this bug is linkml.METAMODEL_CONTEXT_URI.

Currently it evaluates to https://w3id.org/linkml/meta.jsonld

you may notice that doesn't resolve. the correct URL is https://w3id.org/linkml/meta.context.jsonld

Our journey starts here:

METAMODEL_CONTEXT_URI = linkml_files.URL_FOR(Source.META, Format.JSONLD)

up through the latest release, 1.7.5, this would have given us the correct url using the Source and Format enums: https://github.com/linkml/linkml-runtime/blob/b01abb4a4f29b5433a4c0ed3575ac3f607542bbc/linkml_runtime/linkml_model/linkml_files.py#L92

The problem with that version is that python doesn't allow you to have an enum with duplicate values, and the location information was split across two enums so it was a bit error prone.

in linkml-runtime#310 I refactored that so Source and Format are contentless enums, and both the path and suffix information is stored in _Path. SOMEHOW i managed to update all the functions there except for URL_FOR. so now it is incorrectly using the enum rather than _Path.

I actually added a test to ensure that all these URLs resolve: https://github.com/linkml/linkml-runtime/blob/e5d1886e827d7d9d4814535683276546d60abe18/tests/test_linkml_model/test_linkml_files.py#L114-L122 but marked it as skip until we are able to cache network requests, and didn't want to add that there bc i didn't want to do too much in one PR.

THE BRIGHT SIDE: I caught this bug before it was release in linkml-runtime because i just got the action to run upstream tests against linkml-runtime to work - https://github.com/sneakers-the-rat/linkml-runtime/actions/runs/8531019215
and it correctly failed because RDFGenerator tries to resolve the metamodel context URI.

so while i am sorry to have caused this one, i will now work on fixing this one and making bugs like harder to cause in the future

@cmungall
Copy link
Member

cmungall commented Apr 3, 2024

Thanks for the explanation and for catching it! And it's on us in the core to watch out for these, I am the one who approved it!

I have some ideas about how we can be strategic in our refactoring to avoid this kind of thing, if you want to join one of our Friday 1pm dev calls we can discuss...

@sneakers-the-rat
Copy link
Collaborator Author

if we're talking refactoring i am thereeeeee i'll put it on my calendar. every friday?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that should work but isn't, with an example and a test case. linkml-runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants