Add LinkML <-> Ossie ontology converters - #389
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Required dependency approval is not recorded, and minor documentation corrections remain.
Pull request overview
Adds bidirectional LinkML ↔ Ossie ontology converters using neverblink-linkml, with APIs, CLIs, documentation, and tests.
Changes:
- Added both converter directions and public exports.
- Added CLI scripts and dependency locking.
- Added integration and round-trip tests.
File summaries
| File | Summary | Review notes |
|---|---|---|
converters/ontology/uv.lock |
Locks the new dependency. | — |
converters/ontology/tests/test_linkml_converter.py |
Tests conversion and round-trip behavior. | — |
converters/ontology/src/ossie_ontology/converter/ossie_to_linkml/converter.py |
Implements Ossie-to-LinkML conversion. | — |
converters/ontology/src/ossie_ontology/converter/ossie_to_linkml/__init__.py |
Adds converter package initialization. | — |
converters/ontology/src/ossie_ontology/converter/linkml_to_ossie/converter.py |
Implements LinkML-to-Ossie conversion. | — |
converters/ontology/src/ossie_ontology/converter/linkml_to_ossie/__init__.py |
Adds converter package initialization. | — |
converters/ontology/src/ossie_ontology/__init__.py |
Exposes both converters. | — |
converters/ontology/scripts/ossie_to_linkml.py |
Adds the Ossie-to-LinkML CLI. | — |
converters/ontology/scripts/linkml_to_ossie.py |
Adds the LinkML-to-Ossie CLI. | Fix the loader-status sentence grammar. |
converters/ontology/README.md |
Documents converter APIs and CLI usage. | Fix the run-on sentence. |
converters/ontology/pyproject.toml |
Adds the runtime dependency. | PMC/IPMC approval remains to be recorded. |
Review details
Suppressed comments (2)
converters/ontology/README.md:125
- This sentence joins two independent clauses with a comma, making the new usage note read as a run-on sentence. Use a semicolon (or split it) so the documented CLI behavior is clear.
A schema can load and still have errors and warnings against it. Errors and warnings are written to stderr, only fatal problems stop the run.
converters/ontology/pyproject.toml:42
- This adds a new runtime dependency, while the PR checklist explicitly leaves the required PMC/IPMC approval unchecked. Please obtain and record that approval before merging this dependency, which is now required by every installation of the package.
"neverblink-linkml",
- Files reviewed: 10/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
kayemkim
left a comment
There was a problem hiding this comment.
Thanks for bringing this over, and for the mapping doc, which made checking it much easier. Ran this on top of current main: the ontology converter CI steps pass on 3.11 through 3.14, and the flights fixture goes Ossie -> LinkML -> Ossie with both directions exiting 0 and the result validating against ontology/ontology.json. On the dependency, the wheel's metadata says License-Expression: Apache-2.0 with the LICENSE file included, and it declares no Requires-Dist, which matches the description; the approval itself is for the PMC.
One suggestion before this lands. The docstrings, README and test_roundtrip_drops_ontology_mappings say the only loss is ontology_mappings, but a structural diff of the flights round trip shows more, all without a line on stderr:
- all four
derived_by(theaverage_*_delayrelationships on Airport and Route) come back as plain attributes withrange: Delay - the document-level
requires(COUNT[Airport] > 0,COUNT[Carrier] > 0) are gone - the ternary
Flight.registers_latitude_seriesloses itsDegreesLatituderole and comes back binary; the unaryFlight.canceledgains aStringrole - the five relationship-level
requires(Flight.departs_at < Flight.arrives_at,NOT Route.destination(Airport), ...) are gone - two concept-level
requiresare gone because the parser does not read their form:CancelationCode == 'A' OR ... == 'D'(onlyIN (...)is read) and1 <= DistanceGroup(DistanceGroup <= 10survives, the literal-on-the-left one does not)
Your mapping doc does say derived_by, document requires and extra roles are not emitted, so the library is behaving as documented. But someone reading this repo only sees the ontology_mappings caveat, and the round-trip tests compare names, types and identify_by, so they pass through all of the above. Two small additions would close that gap: list these losses next to ontology_mappings in the module docstring and README, and add round-trip tests in the style of test_roundtrip_drops_ontology_mappings for at least derived_by and n-ary roles. palantir_to_ossie in the same package uses warnings.warn when it drops something, so a warning per dropped construct would also fit here if the library can report them.
The expression point connects to #361: which requires forms are read is exactly the kind of thing a grammar would settle, and the repo's own flights fixture uses both the readable and the unreadable spellings. Until then a line in the README saying which forms the importer reads would help.
Nit: scripts/linkml_to_ossie.py lines 26-27 repeat the "loader reports are written to stderr" sentence, the original line was left in when the Copilot fix was applied.
Done, also updated the docstrings of scripts.
Done. Could you re-review the PR? Regarding the warnings and the compatibility in general – oh dear :D it's an extremely complex problem you are touching upon here. We've been working on something very similar in LinkML, so I have some perspective here. In general, schema/ontology languages are not equal. They use different formalisms, have different expressivity and lists of supported features. Any conversion is inherently lossy. With emitting warnings, the issue is both the unclear (unformalized) scope of both Ossie and LinkML, and their broad feature sets. LinkML has hundreds of features (e.g., see the definition of a slot in the metamodel) and the vast majority of them are not directly expressible in Ossie, so we would issue hundreds of warnings. For the opposite direction, it's not really clear what is and is not a feature of Ossie, because it's so young. Proving reliably that feature X is representable in both language A and language B, requires at least some minimal rigor in how each language is defined. We don't have that yet. If you look at how we write and test LinkML-Scala, we use very much a robust, spec-driven approach: we use BNF-derived grammars for parsing, ASTs with case matching for behaviors, we write spec-driven fuzzing tests, and test the conversion on a large catalog of sample schemas. We can get to good bi-di conversion coverage :) but (1) it's never going to be 100%, and (2) it's going to require a lot of formal/semi-formal work in both specifications. Our team would be happy to help with that. |
Summary
Little bit of context: Hi, I'm Piotr, maintainer of one of LinkML implementations: LinkML-Scala. We've been tracking Ossie's progress, and saw a good opportunity to contribute converters for LinkML, as this was already requested by the Ossie community. We'll be more active in the community in the coming weeks :) we plan to start attending the WG meetings.
This PR adds converters for LinkML -> Ossie ontology, and Ossie ontology -> LinkML.
neverblink-linkml.Related Issues
Checklist
Converters
Documentation
docs/is updated to reflect any user-facing changesTests
pytest/ CI green)Compliance
^ needs PMC approval! This should be easy, though, because the only dependency is licensed on Apache 2.0.