-
Notifications
You must be signed in to change notification settings - Fork 74
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
OWLAPI 4.5.24 adds a weird axiom annotation to OBO serialisation #1089
Comments
I think there are three paths forward here:
|
Why does this happen often? To me it looks like an error in the source ontology which should be corrected. |
https://api.triplydb.com/s/dPBG6EPYW The problem is that this never happened before, so the serialiser behaves differently than what it used to. Else we agree. |
From an ongoing discussion on the OBO Slack: As noted in the issue, those “weird annotation axioms” are caused by a faulty AnnotationProperty that should not exist. Should we try to deal gracefully with the issue (that is, not emitting the “weird annotation axioms” in response to the faulty AnnotationProperty)? There are several places where it could be done:
I am personally slightly in favour of option 4. Removing or ignoring faulty OWL statements such as the one in cause here is a workaround that in my opinion has no place in the OWL API, and barely in ROBOT. Hardcoding such a workaround in the OWL API gives no incentives to ontology editors to fix their ontologies. |
I think behavior of the obo writer in 4.5.24 is consistent with other writers, and is consistent with what I think is a bug in the RDF-based parsers. The behavior of the obo writer in 4.5.6 is not consistent with other writers. To see, have a look at any non-RDF serialization of the example ontology above (in either 4.5.6 or in 4.5.24): Functional: AnnotationAssertion(Annotation(oboInOwl:hasDbXref "FlyBase:FBrf0075072") Annotation(rdf:type owl:Axiom) obo:IAO_0000115 obo:FBbt_00000006 "Any segment (FBbt:00000003) that is part of some head (FBbt:00000004).") Manchester: Class: <http://purl.obolibrary.org/obo/FBbt_00000006>
Annotations:
Annotations: <http://www.geneontology.org/formats/oboInOwl#hasDbXref> "FlyBase:FBrf0075072",
rdf:type <http://www.w3.org/2002/07/owl#Axiom>
<http://purl.obolibrary.org/obo/IAO_0000115> "Any segment (FBbt:00000003) that is part of some head (FBbt:00000004).",
<http://www.geneontology.org/formats/oboInOwl#hasDbXref> "UBERON:6000006" This is consistent with the internal OWLAPI representation which can be seen in Protege I think the correct thing to do here is for the OWLAPI to never interpret However, I think a perfectly practical thing to do here is to add in a filter at write-time (Nico's 1 or 2b). This maintains the current "repair magic" in the obo writer and maintains the current status of having different behavior from the ofn and omn writers. Another option is to bless 4.5.24, and just live with the fact that some ontologies will have a one-time diff if they don't repair the error (which might be annoying if it's upstream). However, I am against this, because of another issue, and this one is squarely the fault of the obo writer. If we take the obof output:
and convert it back to OWL we get this monstrosity:
And note this is not parseable by Rust RDF parsers, and This is a known issue that doesn't normally surface as the convention is to use oio for the axiom annotation, and I think fixing this is outside the scope of the next release. So I think that preserving the status quo and implementing Nico's filter is the way to go, the same as Damien's (2). We should still pursue Damien's (4) alongside a battery of other tests we should be running, but this is not so urgent if we can just maintain the status quo. |
I'm curious as to the origin of that annotation property declaration - I wouldn't think someone is trying that on purpose, wondering if it's a bug - and, if so, if it's in the OWL API. Re how to treat it, it's an invalid axiom and as such should cause an error to be thrown in strict parsing mode; in relaxed parsing mode, we've tended to not try anything on such invalid axioms unless they get in the way of rendering data properly - which is the case, here. So I'd go for ignoring this axiom during rendering, at the very least. But ignoring it during parsing requires less code (changes on one parser rather than changes in all renderers). (I haven't tried parsing this example in strict mode - if anyone else has, please let me know, saves me a job) |
@gouttegd - after reading @cmungall comment I have a mild inclination to this practical path:
EDIT: moved list of action items to comment later in the thread. I would prefer right now not to do a repair at OWL API level (dropping axioms) - there may be people that want to use illegal axiom combinations for testing things. ROBOT should only repair when you ask it to do so. |
This could be anything, and is quite prevalent: https://api.triplydb.com/s/dPBG6EPYW Best guess - some RDF outside of OWL was imported at some point into an OWL ontology and that's when it festered.
Strict parsing may be the right option, but I think its too draconian. We cant change ROBOT to strict parsing without breaking too many things unfortunately.. I opt for fixing the renderers, which should have a standard mechanism to handle "unsupported axioms".. Thank you @ignazio1977 for your comments! |
You mean when writing an OBO file? I don’t think that will do any good. When writing to OBO, the problem is not the AnnotationProperty declaration itself (which is already not translated into the OBO file, not even in the |
Ok, after consulting one last time with @gouttegd and @hkir-dev offline, this is, I hope, what we agree on doing:
For now, no repairs anywhere. This is just recovering a state that was working as expected, albeit with a tiny hack on the OBO Format parser. |
Two alternatives are implemented at: |
Excellent @hkir-dev - I will take it from here! THANKS! |
I favor owlcs/owlapi#1092 but you can proceed with either solution as you see best (I edited 1092 to add a comment - I am a big fan of comments pointing back to issues and description of context - if you go with the other PR add a similar comment) |
Seems owlcs/owlapi#1092 was not enough.. Sorry @cmungall I almost merged this into the new ENVO release: Was there anything deeply wrong with owlcs/owlapi#1093? |
Made a call now. There is a tiny risk involved, but we will opt for: |
OWLAPI 1.5.24 adds a weird axiom annotation to OBO serialisation. This was not previously the case and only happens if the ontology is not legal OWL.
Minimal example:
Running this command:
Will result in this OBO file:
While it should be:
The culprit is this axiom:
Which constitutes a violation of the OWL specification, but unfortunately happens rather often.
The text was updated successfully, but these errors were encountered: