Merge pull request #1104 from ontodev/template-cycle-bug

Fix how Template adds entities to the QuotedEntityChecker
ontodev · Nov 29, 2023 · 08b89af · 08b89af
2 parents 2f9eca4 + 3de1c0c
commit 08b89af
Show file tree

Hide file tree

Showing 8 changed files with 184 additions and 90 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix how Template adds entities to the QuotedEntityChecker [#1104]
+
+
 ## [1.9.5] - 2023-09-20
 
 ### Added
@@ -379,6 +384,7 @@ First official release of ROBOT!
 [#1148]: https://github.com/ontodev/robot/pull/1148
 [#1135]: https://github.com/ontodev/robot/pull/1135
 [#1119]: https://github.com/ontodev/robot/pull/1119
+[#1104]: https://github.com/ontodev/robot/pull/1104
 [#1100]: https://github.com/ontodev/robot/pull/1100
 [#1091]: https://github.com/ontodev/robot/issues/1091
 [#1089]: https://github.com/ontodev/robot/issues/1089

diff --git a/docs/extract.md b/docs/extract.md
@@ -79,12 +79,14 @@ For more details see the [MIREOT paper](http://dx.doi.org/10.3233/AO-2011-0087).
 
 The subset method extracts a sub-ontology that contains only the seed terms (that you specify with `--term` and `--term-file` options) and the relations between them. This method uses the [relation-graph](https://github.com/balhoff/relation-graph) to materialize the existential relations among the seed terms. Procedurally, the subset method materializes the input ontology and adds the inferred axioms to the input ontology. Then filters the ontology with the given seed terms. Finally, it reduces the filtered ontology to remove redundant subClassOf axioms.
 
-    robot extract --method subset \
-        --input subset.obo \
-        --term "obo:ONT_1" \
-        --term "obo:ONT_5" \
-        --term "BFO:0000050" \
-        --output results/subset_result.owl
+```
+robot extract --method subset \
+    --input subset.obo \
+    --term "obo:ONT_1" \
+    --term "obo:ONT_5" \
+    --term "BFO:0000050" \
+    --output results/subset_result.owl
+```
         
 ROBOT expects any `--term` or IRI in the `--term-file` to exist in the input ontology. If none of the input terms exist, the command will fail with an [empty terms error](errors#empty-terms-error). This can be overridden by including `--force true`. 
 

diff --git a/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java b/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java
@@ -249,6 +249,9 @@ public void add(OWLEntity entity, String name) {
     if (entity == null) {
       return;
     }
+    if (name == null) {
+      return;
+    }
 
     Map<String, IRI> map = pickMap(entity);
     if (map == null) {

diff --git a/robot-core/src/main/java/org/obolibrary/robot/Template.java b/robot-core/src/main/java/org/obolibrary/robot/Template.java
@@ -173,7 +173,7 @@ public Template(@Nonnull String name, @Nonnull List<List<String>> rows) throws E
 
     // Add the contents of the tableRows
     addTable(rows);
-    addLabels();
+    addEntities();
     createParser();
   }
 
@@ -203,7 +203,7 @@ public Template(@Nonnull String name, @Nonnull List<List<String>> rows, IOHelper
 
     // Add the contents of the tableRows
     addTable(rows);
-    addLabels();
+    addEntities();
     createParser();
   }
 
@@ -237,7 +237,7 @@ public Template(@Nonnull String name, @Nonnull List<List<String>> rows, OWLOntol
 
     // Add the contents of the tableRows
     addTable(rows);
-    addLabels();
+    addEntities();
     createParser();
   }
 
@@ -276,7 +276,7 @@ public Template(
 
     // Add the contents of the tableRows
     addTable(rows);
-    addLabels();
+    addEntities();
     createParser();
   }
 
@@ -320,7 +320,7 @@ public Template(
 
     // Add the contents of the tableRows
     addTable(rows);
-    addLabels();
+    addEntities();
     createParser();
     parser.setOWLEntityChecker(this.checker);
   }
@@ -557,102 +557,103 @@ private void addTable(List<List<String>> rows) throws Exception {
     }
   }
 
-  /** Add the labels from the rows of the template to the QuotedEntityChecker. */
-  private void addLabels() {
-    // If there's no label column, we can't add labels
-    if (labelColumn == -1) {
+  /** Add the entities from the rows of the template to the QuotedEntityChecker. */
+  private void addEntities() {
+    for (List<String> row : tableRows) {
+      addEntity(row);
+    }
+  }
+
+  /** Add the entity from this row of the template to the QuotedEntityChecker. */
+  private void addEntity(List<String> row) {
+    String id = null;
+    try {
+      id = row.get(idColumn);
+    } catch (IndexOutOfBoundsException e) {
+      // ignore
+    }
+
+    if (id == null) {
       return;
     }
-    for (List<String> row : tableRows) {
-      String id = null;
-      if (idColumn != -1) {
-        try {
-          id = row.get(idColumn);
-        } catch (IndexOutOfBoundsException e) {
-          // ignore
-        }
-      }
 
-      String label = null;
+    String label = null;
+    try {
+      label = row.get(labelColumn);
+    } catch (IndexOutOfBoundsException e) {
+      // ignore
+    }
+
+    String type = null;
+    if (typeColumn != -1) {
       try {
-        label = row.get(labelColumn);
+        type = row.get(typeColumn);
       } catch (IndexOutOfBoundsException e) {
         // ignore
       }
+    }
+    if (type == null || type.trim().isEmpty()) {
+      type = "class";
+    }
 
-      if (idColumn != -1 && id == null) {
-        continue;
-      }
-
-      if (id == null || label == null) {
-        continue;
-      }
-
-      String type = null;
-      if (typeColumn != -1) {
-        try {
-          type = row.get(typeColumn);
-        } catch (IndexOutOfBoundsException e) {
-          // ignore
-        }
-      }
-      if (type == null || type.trim().isEmpty()) {
-        type = "class";
-      }
+    IRI iri = ioHelper.createIRI(id);
+    if (iri == null) {
+      iri = IRI.create(id);
+    }
 
-      IRI iri = ioHelper.createIRI(id);
-      if (iri == null) {
-        iri = IRI.create(id);
-      }
+    // Try to resolve a CURIE
+    IRI typeIRI = ioHelper.createIRI(type);
 
-      // Try to resolve a CURIE
-      IRI typeIRI = ioHelper.createIRI(type);
+    // Set to IRI string or to type string
+    String typeOrIRI = type;
+    if (typeIRI != null) {
+      typeOrIRI = typeIRI.toString();
+    }
 
-      // Set to IRI string or to type string
-      String typeOrIRI = type;
-      if (typeIRI != null) {
-        typeOrIRI = typeIRI.toString();
-      }
+    // Check against builtin types (ignore case), otherwise treat as individual
+    OWLEntity entity;
+    String lowerCaseType = typeOrIRI.toLowerCase();
+    switch (lowerCaseType) {
+      case "":
+      case "http://www.w3.org/2002/07/owl#class":
+      case "class":
+        entity = dataFactory.getOWLEntity(EntityType.CLASS, iri);
+        break;
 
-      // Check against builtin types (ignore case), otherwise treat as individual
-      OWLEntity entity;
-      String lowerCaseType = typeOrIRI.toLowerCase();
-      switch (lowerCaseType) {
-        case "":
-        case "http://www.w3.org/2002/07/owl#class":
-        case "class":
-          entity = dataFactory.getOWLEntity(EntityType.CLASS, iri);
-          break;
+      case "http://www.w3.org/2002/07/owl#objectproperty":
+      case "object property":
+        entity = dataFactory.getOWLEntity(EntityType.OBJECT_PROPERTY, iri);
+        break;
 
-        case "http://www.w3.org/2002/07/owl#objectproperty":
-        case "object property":
-          entity = dataFactory.getOWLEntity(EntityType.OBJECT_PROPERTY, iri);
-          break;
+      case "http://www.w3.org/2002/07/owl#dataproperty":
+      case "data property":
+        entity = dataFactory.getOWLEntity(EntityType.DATA_PROPERTY, iri);
+        break;
 
-        case "http://www.w3.org/2002/07/owl#dataproperty":
-        case "data property":
-          entity = dataFactory.getOWLEntity(EntityType.DATA_PROPERTY, iri);
-          break;
+      case "http://www.w3.org/2002/07/owl#annotationproperty":
+      case "annotation property":
+        entity = dataFactory.getOWLEntity(EntityType.ANNOTATION_PROPERTY, iri);
+        break;
 
-        case "http://www.w3.org/2002/07/owl#annotationproperty":
-        case "annotation property":
-          entity = dataFactory.getOWLEntity(EntityType.ANNOTATION_PROPERTY, iri);
-          break;
+      case "http://www.w3.org/2002/07/owl#datatype":
+      case "datatype":
+        entity = dataFactory.getOWLEntity(EntityType.DATATYPE, iri);
+        break;
 
-        case "http://www.w3.org/2002/07/owl#datatype":
-        case "datatype":
-          entity = dataFactory.getOWLEntity(EntityType.DATATYPE, iri);
-          break;
+      case "http://www.w3.org/2002/07/owl#individual":
+      case "individual":
+      case "http://www.w3.org/2002/07/owl#namedindividual":
+      case "named individual":
+      default:
+        // Assume type is an individual (checked later)
+        entity = dataFactory.getOWLEntity(EntityType.NAMED_INDIVIDUAL, iri);
+        break;
+    }
 
-        case "http://www.w3.org/2002/07/owl#individual":
-        case "individual":
-        case "http://www.w3.org/2002/07/owl#namedindividual":
-        case "named individual":
-        default:
-          // Assume type is an individual (checked later)
-          entity = dataFactory.getOWLEntity(EntityType.NAMED_INDIVIDUAL, iri);
-          break;
-      }
+    if (id != null) {
+      checker.add(entity, id);
+    }
+    if (label != null) {
       checker.add(entity, label);
     }
   }
@@ -795,6 +796,8 @@ private void processRow(List<String> row) throws Exception {
       case "http://www.w3.org/2002/07/owl#namedindividual":
       case "named individual":
       default:
+        // This is a bit unsafe imo, for example in the case of where the datatype turns out to be
+        // http://www.w3.org/2002/07/owl#DatatypeProperty""
         addIndividualAxioms(iri, row);
         break;
     }

diff --git a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java
@@ -49,6 +49,35 @@ public void testLegacyTemplateCSV() throws Exception {
     assertIdentical("/template.owl", template);
   }
 
+  /**
+   * Test a strange case where a sequence .
+   *
+   * @throws Exception if entities cannot be found
+   */
+  @Test
+  public void testNoLabelsNoTypes() throws Exception {
+    String path = "/sequence-template.csv";
+    List<List<String>> rows = TemplateHelper.readCSV(this.getClass().getResourceAsStream(path));
+    Template t = new Template(path, rows);
+    t.generateOutputOntology("http://test.com/template.owl", false, null);
+  }
+
+  /**
+   * Test a strange case where a sequence .
+   *
+   * @throws Exception if entities cannot be found
+   */
+  @Test
+  public void testNoLabels() throws Exception {
+    String path = "/workflow-template.csv";
+    List<List<String>> rows = TemplateHelper.readCSV(this.getClass().getResourceAsStream(path));
+    IOHelper ioHelper = new IOHelper();
+    ioHelper.addPrefix("ex", "http://example.com/");
+    Template t = new Template(path, rows, ioHelper);
+    OWLOntology template = t.generateOutputOntology("http://test.com/template.owl", false, null);
+    assertIdentical("/workflow-template.ttl", template);
+  }
+
   /**
    * Test multiple templates.
    *

diff --git a/robot-core/src/test/resources/sequence-template.csv b/robot-core/src/test/resources/sequence-template.csv
@@ -0,0 +1,4 @@
+ID,isa
+ID,SC %
+CL:4030028,CL:0000561
+CL:0000561,CL:0000099
diff --git a/robot-core/src/test/resources/workflow-template.csv b/robot-core/src/test/resources/workflow-template.csv
@@ -0,0 +1,5 @@
+Identifier,Type,Class Restriction Lower,Class Restriction Upper,Domain,Range
+ID,TYPE,SC %,SC %,DOMAIN,RANGE
+ex:consists_of,owl:ObjectProperty,,,ex:Workflow,ex:Task
+ex:Workflow,class,(ex:consists_of min 1 ex:Task),(ex:consists_of max 7 ex:Task),,
+ex:Task,class,,,,
diff --git a/robot-core/src/test/resources/workflow-template.ttl b/robot-core/src/test/resources/workflow-template.ttl
@@ -0,0 +1,42 @@
+@prefix owl: <http://www.w3.org/2002/07/owl#> .
+@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> .
+@prefix xml: <http://www.w3.org/XML/1998/namespace> .
+@prefix xsd: <http://www.w3.org/2001/XMLSchema#> .
+@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> .
+@base <http://www.w3.org/2002/07/owl#> .
+
+<http://test.com/template.owl> rdf:type owl:Ontology .
+
+#################################################################
+#    Object Properties
+#################################################################
+
+###  http://example.com/consists_of
+<http://example.com/consists_of> rdf:type owl:ObjectProperty ;
+                                       rdfs:domain <http://example.com/Workflow> ;
+                                       rdfs:range <http://example.com/Task> .
+
+
+#################################################################
+#    Classes
+#################################################################
+
+###  http://example.com/Task
+<http://example.com/Task> rdf:type owl:Class .
+
+
+###  http://example.com/Workflow
+<http://example.com/Workflow> rdf:type owl:Class ;
+                                    rdfs:subClassOf [ rdf:type owl:Restriction ;
+                                                      owl:onProperty <http://example.com/consists_of> ;
+                                                      owl:minQualifiedCardinality "1"^^xsd:nonNegativeInteger ;
+                                                      owl:onClass <http://example.com/Task>
+                                                    ] ,
+                                                    [ rdf:type owl:Restriction ;
+                                                      owl:onProperty <http://example.com/consists_of> ;
+                                                      owl:maxQualifiedCardinality "7"^^xsd:nonNegativeInteger ;
+                                                      owl:onClass <http://example.com/Task>
+                                                    ] .
+
+
+###  Generated by the OWL API (version 4.5.26) https://github.com/owlcs/owlapi