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

Should genusIds be optional in logicalDefinitionAxioms? #89

Closed
matentzn opened this issue Dec 5, 2022 · 12 comments
Closed

Should genusIds be optional in logicalDefinitionAxioms? #89

matentzn opened this issue Dec 5, 2022 · 12 comments

Comments

@matentzn
Copy link

matentzn commented Dec 5, 2022

Currently, we cant open several obographs json files, see also monarch-initiative/MAxO#362, because of this error:

 runoak -i prontolib:maxo.json validate
ValueError: graphs[0].logicalDefinitionAxioms[0]: missing field `genusIds` at line 212757 column 9

Here is an extract of the related section in the ontology:

"id": "http://purl.obolibrary.org/obo/maxo.json",
      "logicalDefinitionAxioms": [
        {
          "definedClassId": "http://purl.obolibrary.org/obo/HP_0000532"
        },
        {
          "definedClassId": "http://purl.obolibrary.org/obo/HP_0000533"
        },

What is the right thing to do here?

  • Declare genusIds as optional, so parsers should skip it
  • DO not serialise definitions without genus ids, like all the ones in HPO
@cmungall
Copy link
Member

cmungall commented Dec 5, 2022

2

@julesjacobsen
Copy link
Collaborator

Can you provide a chunk of OWL to test this. Also, I presume its ok for the restrictions to be empty?

cmungall added a commit to INCATools/ontology-access-kit that referenced this issue Dec 5, 2022
cmungall added a commit to INCATools/ontology-access-kit that referenced this issue Dec 5, 2022
* Adding documentation for logical definition axioms in obograph.

See geneontology/obographs#89

* Enhancements for ontology statistics

Including contributor statistics, fixes #378

Allowing diff statistics to be folded into general statistics

* lint
@matentzn
Copy link
Author

matentzn commented Dec 7, 2022

@julesjacobsen here is a junk of OWL:
https://raw.githubusercontent.com/monarch-ebi-dev/robot_tests/master/data/examples-logicaldefinitionaxioms.owl.rdf

I presume this:

  • C1: V = R some A: No genus, do not include at all in logicalDefinitionAxioms
  • C2: W = E: This should be included, but you need to make sure the spec says where
  • C3: X = A and (R some (S some B)): Complex axiom, need to check with @cmungall what needs to happen here
  • C4: Y = A and (R some B): Definetly needs to be included
  • C5: Z = A or B: Need to check with @cmungall

Also, I presume its ok for the restrictions to be empty?

I am assuming you are asking about C2?

@joeflack4
Copy link

joeflack4 commented Dec 9, 2022

@julesjacobsen I have some OWL below you can look at. I had opened up an issue in robot (ontodev/robot#1079), and Nico felt the root of my issue lies here, with Obographs. Below is a copy/paste of what I wrote.

Although I had thought that this issue was not pertaining specifically to genusIds, but axioms about them that have nested owl:Restrictions. I see that all cases of the issue I'm encountering are indeed about such genusId-related axioms.


Overview

I am converting an OWL file to Obographs JSON. I was then using that JSON for something else, and I encountered an error because null was not anticipated. I'm not sure if this is (a) a bug, (b) a case of something in OWL that is not supported by Obographs / intended not to be serialized, and therefore null is intended here, (c) intended by robot for some other reason.

Command

robot convert -i INPUT_OWL_PATH -o OUTPUT_OBOGRAPHS_PATH --format json

Input

Snippet of the hpo.owl where the source of null came from. Namely, it is a case of an owl:Restriction within another owl:Restriction.

Sub-snippet:

                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000051"/>
                        <owl:someValuesFrom>
                            <owl:Restriction>
                                <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000050"/>
                                <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0009768"/>
                            </owl:Restriction>
                        </owl:someValuesFrom>
                    </owl:Restriction>

The full axiom:

    <owl:Axiom>
        <owl:annotatedSource rdf:resource="http://purl.obolibrary.org/obo/UBERON_0009551"/>
        <owl:annotatedProperty rdf:resource="http://www.w3.org/2002/07/owl#equivalentClass"/>
        <owl:annotatedTarget>
            <owl:Class>
                <owl:intersectionOf rdf:parseType="Collection">
                    <rdf:Description rdf:about="http://purl.obolibrary.org/obo/UBERON_0002529"/>
                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000050"/>
                        <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0002544"/>
                    </owl:Restriction>
                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000051"/>
                        <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0004300"/>
                    </owl:Restriction>
                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000051"/>
                        <owl:someValuesFrom>
                            <owl:Restriction>
                                <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000050"/>
                                <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0009768"/>
                            </owl:Restriction>
                        </owl:someValuesFrom>
                    </owl:Restriction>
                </owl:intersectionOf>
            </owl:Class>
        </owl:annotatedTarget>
        <oboInOwl:source>cjm</oboInOwl:source>
    </owl:Axiom>

Output

Within logicalDefinitionAxioms. The case of owl:Restriction within another owl:Restriction is null here:

    {
      "definedClassId" : "http://purl.obolibrary.org/obo/UBERON_0009551",
      "genusIds" : [ "http://purl.obolibrary.org/obo/UBERON_0002529" ],
      "restrictions" : [ {
        "propertyId" : "http://purl.obolibrary.org/obo/BFO_0000050",
        "fillerId" : "http://purl.obolibrary.org/obo/UBERON_0002544"
      }, {
        "propertyId" : "http://purl.obolibrary.org/obo/BFO_0000051",
        "fillerId" : "http://purl.obolibrary.org/obo/UBERON_0004300"
      }, null ]
    }

@matentzn
Copy link
Author

@joeflack4 your issue and the genusId issue are related insofar as we need a consistent way to reflect equivalent class axioms. Either we say: restrictions of the type X are too complex and not included. Or we find a way to represent them consistently.

cmungall added a commit to INCATools/ontology-access-kit that referenced this issue Dec 16, 2022
This is consistent with JSON files found in the wild.

See also geneontology/obographs#89

and linkml/linkml#1156
@cmungall
Copy link
Member

In INCATools/ontology-access-kit#414 I am making genusIds recommended rather than required, as it's important to be consistent with json already released. However I think it is better to capture these axioms differently

cmungall added a commit to INCATools/ontology-access-kit that referenced this issue Dec 16, 2022
…nal (#414)

* Added allValuesFromEdges to OBO Graph data model.

Note these are currently treated distinct from the core
edges, which is intentional. Graph operations should
typically not operate over universal restrictions.

* Made genusIds optional in logical definition axioms.
This is consistent with JSON files found in the wild.

See also geneontology/obographs#89

and linkml/linkml#1156

* make restrictions recommended

* Adding test

* placing AVF under domaonRangeAxioms

* update maxo notebook

* temp skip translator test as service unavailable

* allow for case insensitive search

* allow for case insensitive search

* remove print

* annotator whole text match should be case insensitive

* lint

* lint
@julesjacobsen
Copy link
Collaborator

julesjacobsen commented Jan 4, 2023

@joeflack4 @cmungall @matentzn

I'm failing to reproduce this null in the JSON (or YAML) output. Are you sure its an obographs issue? At least this test produces no nulls:

<?xml version="1.0"?>
<rdf:RDF xmlns="http://purl.obolibrary.org/obo/uberon.owl#"
     xml:base="http://purl.obolibrary.org/obo/uberon.owl"
     xmlns:obo="http://purl.obolibrary.org/obo/"
     xmlns:oboInOwl="http://www.geneontology.org/formats/oboInOwl#"
     xmlns:owl="http://www.w3.org/2002/07/owl#"
     xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#">
    <owl:Ontology rdf:about="http://purl.obolibrary.org/obo/uberon.owl">
        <owl:versionIRI rdf:resource="http://purl.obolibrary.org/obo/uberon/releases/2022-12-13/uberon.owl"/>
    </owl:Ontology>
    
<owl:Axiom>
        <owl:annotatedSource rdf:resource="http://purl.obolibrary.org/obo/UBERON_0009551"/>
        <owl:annotatedProperty rdf:resource="http://www.w3.org/2002/07/owl#equivalentClass"/>
        <owl:annotatedTarget>
            <owl:Class>
                <owl:intersectionOf rdf:parseType="Collection">
                    <rdf:Description rdf:about="http://purl.obolibrary.org/obo/UBERON_0002529"/>
                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000050"/>
                        <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0002544"/>
                    </owl:Restriction>
                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000051"/>
                        <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0004300"/>
                    </owl:Restriction>
                    <owl:Restriction>
                        <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000051"/>
                        <owl:someValuesFrom>
                            <owl:Restriction>
                                <owl:onProperty rdf:resource="http://purl.obolibrary.org/obo/BFO_0000050"/>
                                <owl:someValuesFrom rdf:resource="http://purl.obolibrary.org/obo/UBERON_0009768"/>
                            </owl:Restriction>
                        </owl:someValuesFrom>
                    </owl:Restriction>
                </owl:intersectionOf>
            </owl:Class>
        </owl:annotatedTarget>
        <oboInOwl:source>cjm</oboInOwl:source>
    </owl:Axiom>
</rdf:RDF>

when converted to JSON using

OWLOntologyManager m = OWLManager.createOWLOntologyManager();
OWLOntology ontology = m.loadOntologyFromOntologyDocument(new ByteArrayInputStream(axiom.getBytes(StandardCharsets.UTF_8)));
GraphDocument graphDocument = new FromOwl().generateGraphDocument(ontology);
System.out.println(OgJsonGenerator.render(graphDocument));

gives me

{
  "graphs" : [ {
    "id" : "http://purl.obolibrary.org/obo/uberon.owl",
    "meta" : {
      "version" : "http://purl.obolibrary.org/obo/uberon/releases/2022-12-13/uberon.owl"
    },
    "logicalDefinitionAxioms" : [ {
      "definedClassId" : "http://purl.obolibrary.org/obo/UBERON_0009551",
      "genusIds" : [ "http://purl.obolibrary.org/obo/UBERON_0002529" ],
      "restrictions" : [ {
        "propertyId" : "http://purl.obolibrary.org/obo/BFO_0000050",
        "fillerId" : "http://purl.obolibrary.org/obo/UBERON_0002544"
      }, {
        "propertyId" : "http://purl.obolibrary.org/obo/BFO_0000051",
        "fillerId" : "http://purl.obolibrary.org/obo/UBERON_0004300"
      } ]
    } ]
  } ]
}

i.e. no null present. Similarly removing the line

<rdf:Description rdf:about="http://purl.obolibrary.org/obo/UBERON_0002529"/>

results in this JSON output:

{
  "graphs" : [ {
    "id" : "http://purl.obolibrary.org/obo/uberon.owl",
    "meta" : {
      "version" : "http://purl.obolibrary.org/obo/uberon/releases/2022-12-13/uberon.owl"
    },
    "logicalDefinitionAxioms" : [ {
      "definedClassId" : "http://purl.obolibrary.org/obo/UBERON_0009551",
      "restrictions" : [ {
        "propertyId" : "http://purl.obolibrary.org/obo/BFO_0000050",
        "fillerId" : "http://purl.obolibrary.org/obo/UBERON_0002544"
      }, {
        "propertyId" : "http://purl.obolibrary.org/obo/BFO_0000051",
        "fillerId" : "http://purl.obolibrary.org/obo/UBERON_0004300"
      } ]
    } ]
  } ]
}

No null where the genusIds would be...

Could it be something to do with the way ROBOT is loading the ontology?

@joeflack4
Copy link

I thought I had the newest version, but it looks like I was using 1.8.1 and not 1.9+. I know I used a pretty simple command to do this, though. Sorry I don't have more time to look into at the moment. Feel free to close and I can reopen if still an issue.

@julesjacobsen
Copy link
Collaborator

julesjacobsen commented Jan 5, 2023

I think we can close this.

Summary

No changes to be made to obographs.

NULL in JSON output

obographs 0.3.0+ will not write out null to JSON or YAML. If these are being produced by ROBOT then the ROBOT version should be updated to v1.8.4+ which uses obographs 0.3.0.

Missing genusIds

These are now 'recommended' rather than 'required':

In INCATools/ontology-access-kit#414 I am making genusIds recommended rather than required, as it's important to be consistent with json already released. However I think it is better to capture these axioms differently

Nested owl:Restriction

These can't be expressed in obographs JSON/YAML as far as I can see as the ExistentialRestrictionExpression only has propertyId and fillerId fields, unless the meta object can be used. n.b. in this case the nested restriction f is anonymous, so a null is returned in place of an ExistentialRestrictionExpression instance.

@Nullable
private ExistentialRestrictionExpression getRestriction(OWLClassExpression x) {
if (x instanceof OWLObjectSomeValuesFrom) {
OWLObjectSomeValuesFrom r = (OWLObjectSomeValuesFrom) x;
OWLPropertyExpression p = r.getProperty();
OWLClassExpression f = r.getFiller();
if (p instanceof OWLObjectProperty && !f.isAnonymous()) {
return new ExistentialRestrictionExpression.Builder()
.propertyId(getPropertyId((OWLObjectProperty) p))
.fillerId(getClassId((OWLClass) f))
.build();
}
// n.b nested OWLObjectSomeValuesFrom will be removed from the output. These can be found by testing that
// f instanceof OWLObjectSomeValuesFrom
}
return null;
}

@joeflack4
Copy link

Thank you @julesjacobsen. I'm confused on 1 point though; you mentioned that there should never be instances of null in Obographs 0.3.0+, but it looks like this does happen in the case of a nested owl:Restriction? I guess the null returned from that function does not equate to null in the output; the nested OWLObjectSomeValuesFrom will simply be removed from / not appear in the output?

@julesjacobsen
Copy link
Collaborator

@joeflack4 yes, the function returns a null, but this is filtered out further up the stack and won't appear in the output files.

@matentzn if you're satisfied with all this, please close...

@matentzn
Copy link
Author

matentzn commented Jan 9, 2023

@julesjacobsen thank you for working on this :). So basically, to address monarch-initiative/MAxO#362, we need to change OAK/Pronto!

@matentzn matentzn closed this as not planned Won't fix, can't repro, duplicate, stale Jan 9, 2023
@matentzn matentzn closed this as completed Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants