OBOgraphs fails hard in the presence of complex property expression

[matentzn]

Minimal test:

<?xml version="1.0"?>
<rdf:RDF xmlns="http://purl.obolibrary.org/obo/fail-expression.owl#"
     xml:base="http://purl.obolibrary.org/obo/fail-expression.owl"
     xmlns:owl="http://www.w3.org/2002/07/owl#"
     xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
     xmlns:xml="http://www.w3.org/XML/1998/namespace"
     xmlns:xsd="http://www.w3.org/2001/XMLSchema#"
     xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#">
    <owl:Ontology rdf:about="http://purl.obolibrary.org/obo/fail-expression.owl">
    </owl:Ontology>

    <owl:Class rdf:about="http://purl.obolibrary.org/obo/OBI_0000260">

        <rdfs:subClassOf>
            <owl:Restriction>
                <owl:onProperty>
                    <rdf:Description>
                        <owl:inverseOf rdf:resource="http://purl.obolibrary.org/obo/COB_0000087"/>
                    </rdf:Description>
                </owl:onProperty>
                <owl:allValuesFrom rdf:resource="http://purl.obolibrary.org/obo/COB_0000082"/>
            </owl:Restriction>
        </rdfs:subClassOf>
    </owl:Class>

</rdf:RDF>

This fails because of:

<owl:onProperty>
    <rdf:Description>
        <owl:inverseOf rdf:resource="http://purl.obolibrary.org/obo/COB_0000087"/>
    </rdf:Description>
</owl:onProperty>

The rare case where we use property expressions instead of properties.

[matentzn]

@julesjacobsen can you take care of this?

[julesjacobsen]

What is the expected behaviour for the test to pass?

[matentzn]

Imo the unsupported construct should be ignored (logger.warning) and serialisation resumed without it. (Non draconian error handling)

[julesjacobsen]

OK, but instead of ignoring it in the output and logging an error, would this make sense:

---
graphs:
- id: "http://purl.obolibrary.org/obo/fail-expression.owl"
  domainRangeAxioms:
  - predicateId: "http://purl.obolibrary.org/obo/COB_0000087"
    allValuesFromEdges:
    - sub: "http://purl.obolibrary.org/obo/OBI_0000260"
      pred: "inverseOf"
      obj: "http://purl.obolibrary.org/obo/COB_0000087"

[cmungall]

no, this would translate to

OBI_0000260 subClassOf inverseOf only COB_0000087

This should fall into a general bucket where we can provide a generic translation, see #94

[julesjacobsen]

ahh, so these go into the untranslatedAxioms list in the FromOwl class? These are only ever collected and never used afterwards.

In this case the untranslated axioms would be:

Untranslated axioms: [SubClassOf(<http://purl.obolibrary.org/obo/OBI_0000260> ObjectAllValuesFrom(ObjectInverseOf(<http://purl.obolibrary.org/obo/COB_0000087>) <http://purl.obolibrary.org/obo/COB_0000082>))]

There are a lot of guards to check that the entity is named (here is one of them), so should anything failing this be added to the untranslatedAxioms list?

e.g.

else if (supc instanceof OWLObjectAllValuesFrom) {
    OWLObjectAllValuesFrom avf = (OWLObjectAllValuesFrom) supc;
    OWLObjectPropertyExpression property = avf.getProperty();
    if (property instanceof OWLObjectProperty) {
        DomainRangeAxiom.Builder b = getDRBuilder(property, domainRangeBuilderMap);
        if (avf.getFiller().isNamed()) {
            Edge e = getEdge(subj,
                    //TODO CHECK!!!
                    b.build().getPredicateId(),
                    getClassId(avf.getFiller().asOWLClass()),
                    nullIfEmpty(meta));
            b.addAllValuesFromEdge(e);
        } else {
// this wasn't present before
            untranslatedAxioms.add(sca);
        }
    } else if (property instanceof OWLObjectInverseOf) {
// This is new but might be incorrect  OWLObjectAllValuesFrom can either be a OWLObjectProperty or a OWLObjectInverseOf. 
// OWLObjectInverseOf wasn't handled before which is leading to the error
        OWLObjectInverseOf iop = (OWLObjectInverseOf) property;
        if (avf.getFiller().isNamed() && iop.isNamed()) {
            String pid = getPropertyId(iop.getInverse().asOWLObjectProperty());
            DomainRangeAxiom.Builder b = getDRBuilder(iop.getInverse(), domainRangeBuilderMap);
            Edge edge = getEdge(subj, INVERSE_OF, pid, meta);
            b.addAllValuesFromEdge(edge);
        } else {
            // this wasn't present before
           untranslatedAxioms.add(sca);
        }
    }
}

[matentzn]

Needs @cmungall but && iop.isNamed() is exactly what is needed I think; not sure about the avf.getFiller().isNamed() but I guess this is one of the current obographs limitations, so yeah, looks good. Can you make a PR? I will get chris to review this.

[julesjacobsen]

At present, in my working branch, the output is like so

---
graphs:
- id: "http://purl.obolibrary.org/obo/fail-expression.owl"
  nodes:
  - id: "http://purl.obolibrary.org/obo/OBI_0000260"
    type: "CLASS"

with the following being logged to the console

12:26:54.724 [main] WARN  o.g.obographs.owlapi.FromOwl - http://purl.obolibrary.org/obo/fail-expression.owl contains 1 Untranslated axioms:
12:26:54.732 [main] WARN  o.g.obographs.owlapi.FromOwl - SubClassOf(<http://purl.obolibrary.org/obo/OBI_0000260> ObjectAllValuesFrom(ObjectInverseOf(<http://purl.obolibrary.org/obo/COB_0000087>) <http://purl.obolibrary.org/obo/COB_0000082>))

@matentzn , @cmungall does this seem acceptable?

[matentzn]

I love it!

[julesjacobsen]

@matentzn @cmungall @balhoff The PR #98 needs approval for this to be closed.