From e57160fe64cb5a0ec0de7c62de884b6ba5fe65b6 Mon Sep 17 00:00:00 2001 From: Nico Matentzoglu Date: Tue, 21 Mar 2023 17:28:03 +0200 Subject: [PATCH 1/8] This adds a failing test for the template cycle bug --- .../java/org/obolibrary/robot/TemplateTest.java | 14 ++++++++++++++ robot-core/src/test/resources/cycle-template.csv | 4 ++++ 2 files changed, 18 insertions(+) create mode 100644 robot-core/src/test/resources/cycle-template.csv diff --git a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java index 8e39eca03..d4c63f758 100644 --- 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,20 @@ public void testLegacyTemplateCSV() throws Exception { assertIdentical("/template.owl", template); } + /** + * Test legacy templating. + * + * @throws Exception if entities cannot be found + */ + @Test + public void testLegacyTemplateCSV2() throws Exception { + String path = "/cycle-template.csv"; + List> rows = TemplateHelper.readCSV(this.getClass().getResourceAsStream(path)); + Template t = new Template(path, rows); + OWLOntology template = t.generateOutputOntology("http://test.com/template.owl", false, null); + assertIdentical("/template.owl", template); + } + /** * Test multiple templates. * diff --git a/robot-core/src/test/resources/cycle-template.csv b/robot-core/src/test/resources/cycle-template.csv new file mode 100644 index 000000000..d979b6ddb --- /dev/null +++ b/robot-core/src/test/resources/cycle-template.csv @@ -0,0 +1,4 @@ +ID,isa +ID,SC % +CL:4030028,CL:0000561 +CL:0000561,CL:0000099 From b05e531843e87a6b02349521663286d4d2ab7f22 Mon Sep 17 00:00:00 2001 From: Nico Matentzoglu Date: Tue, 21 Mar 2023 23:19:52 +0200 Subject: [PATCH 2/8] Phase 2 debugging of template error --- .../java/org/obolibrary/robot/QuotedEntityChecker.java | 4 ++++ .../src/main/java/org/obolibrary/robot/Template.java | 2 ++ .../src/test/java/org/obolibrary/robot/TemplateTest.java | 9 ++++----- .../{cycle-template.csv => sequence-template.csv} | 0 4 files changed, 10 insertions(+), 5 deletions(-) rename robot-core/src/test/resources/{cycle-template.csv => sequence-template.csv} (100%) diff --git a/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java b/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java index dc81dc6f1..b28f66000 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java +++ b/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java @@ -390,6 +390,10 @@ public OWLDataProperty getOWLDataProperty(@Nonnull String name) { return dataFactory.getOWLDataProperty(iri); } // prevent punning + // This where the problem happens; whenever this method is called, essentially, the incoming + // name gets put into the dataProperties dictionary, no matter what. But this method is called in many cases, for + // example, and most importantly during mancherster syntax parsing. + // However this error seems to have masked a number of other problems; if (!objectProperties.containsKey(name)) { if (ioHelper != null) { iri = ioHelper.createIRI(name); diff --git a/robot-core/src/main/java/org/obolibrary/robot/Template.java b/robot-core/src/main/java/org/obolibrary/robot/Template.java index f98b331fa..e7bc6cda1 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/Template.java +++ b/robot-core/src/main/java/org/obolibrary/robot/Template.java @@ -795,6 +795,8 @@ private void processRow(List 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 index d4c63f758..fbac54fa6 100644 --- a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java +++ b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java @@ -50,17 +50,16 @@ public void testLegacyTemplateCSV() throws Exception { } /** - * Test legacy templating. + * Test a strange case where a sequence . * * @throws Exception if entities cannot be found */ @Test - public void testLegacyTemplateCSV2() throws Exception { - String path = "/cycle-template.csv"; + public void testLegacyTemplateCSV3() throws Exception { + String path = "/sequence-template.csv"; List> rows = TemplateHelper.readCSV(this.getClass().getResourceAsStream(path)); Template t = new Template(path, rows); - OWLOntology template = t.generateOutputOntology("http://test.com/template.owl", false, null); - assertIdentical("/template.owl", template); + t.generateOutputOntology("http://test.com/template.owl", false, null); } /** diff --git a/robot-core/src/test/resources/cycle-template.csv b/robot-core/src/test/resources/sequence-template.csv similarity index 100% rename from robot-core/src/test/resources/cycle-template.csv rename to robot-core/src/test/resources/sequence-template.csv From d79fb78f93220b93e8bec652a21d379fc9e3691e Mon Sep 17 00:00:00 2001 From: "James A. Overton" Date: Tue, 28 Nov 2023 16:52:26 -0500 Subject: [PATCH 3/8] Add all entities to checker --- .../obolibrary/robot/QuotedEntityChecker.java | 7 ++-- .../java/org/obolibrary/robot/Template.java | 32 ++++++++----------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java b/robot-core/src/main/java/org/obolibrary/robot/QuotedEntityChecker.java index b28f66000..e5840fffe 100644 --- 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 map = pickMap(entity); if (map == null) { @@ -390,10 +393,6 @@ public OWLDataProperty getOWLDataProperty(@Nonnull String name) { return dataFactory.getOWLDataProperty(iri); } // prevent punning - // This where the problem happens; whenever this method is called, essentially, the incoming - // name gets put into the dataProperties dictionary, no matter what. But this method is called in many cases, for - // example, and most importantly during mancherster syntax parsing. - // However this error seems to have masked a number of other problems; if (!objectProperties.containsKey(name)) { if (ioHelper != null) { iri = ioHelper.createIRI(name); diff --git a/robot-core/src/main/java/org/obolibrary/robot/Template.java b/robot-core/src/main/java/org/obolibrary/robot/Template.java index e7bc6cda1..21aeb8d51 100644 --- a/robot-core/src/main/java/org/obolibrary/robot/Template.java +++ b/robot-core/src/main/java/org/obolibrary/robot/Template.java @@ -559,33 +559,23 @@ private void addTable(List> 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) { - return; - } for (List row : tableRows) { String id = null; - if (idColumn != -1) { - try { - id = row.get(idColumn); - } catch (IndexOutOfBoundsException e) { - // ignore - } - } - - String label = null; try { - label = row.get(labelColumn); + id = row.get(idColumn); } catch (IndexOutOfBoundsException e) { // ignore } - if (idColumn != -1 && id == null) { + if (id == null) { continue; } - if (id == null || label == null) { - continue; + String label = null; + try { + label = row.get(labelColumn); + } catch (IndexOutOfBoundsException e) { + // ignore } String type = null; @@ -653,7 +643,13 @@ private void addLabels() { entity = dataFactory.getOWLEntity(EntityType.NAMED_INDIVIDUAL, iri); break; } - checker.add(entity, label); + + if (id != null) { + checker.add(entity, id); + } + if (label != null) { + checker.add(entity, label); + } } } From 99e4e47a33f970a8a0a95c59bff428b02b154abe Mon Sep 17 00:00:00 2001 From: "James A. Overton" Date: Tue, 28 Nov 2023 17:14:06 -0500 Subject: [PATCH 4/8] Add test based on #1133 --- .../org/obolibrary/robot/TemplateTest.java | 16 +++++++ .../src/test/resources/workflow-template.csv | 5 +++ .../src/test/resources/workflow-template.ttl | 42 +++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 robot-core/src/test/resources/workflow-template.csv create mode 100644 robot-core/src/test/resources/workflow-template.ttl diff --git a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java index fbac54fa6..c3233d4b5 100644 --- a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java +++ b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java @@ -62,6 +62,22 @@ public void testLegacyTemplateCSV3() throws Exception { 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> 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/workflow-template.csv b/robot-core/src/test/resources/workflow-template.csv new file mode 100644 index 000000000..f636bf563 --- /dev/null +++ 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 new file mode 100644 index 000000000..3ab680dba --- /dev/null +++ b/robot-core/src/test/resources/workflow-template.ttl @@ -0,0 +1,42 @@ +@prefix owl: . +@prefix rdf: . +@prefix xml: . +@prefix xsd: . +@prefix rdfs: . +@base . + + rdf:type owl:Ontology . + +################################################################# +# Object Properties +################################################################# + +### http://example.com/consists_of + rdf:type owl:ObjectProperty ; + rdfs:domain ; + rdfs:range . + + +################################################################# +# Classes +################################################################# + +### http://example.com/Task + rdf:type owl:Class . + + +### http://example.com/Workflow + rdf:type owl:Class ; + rdfs:subClassOf [ rdf:type owl:Restriction ; + owl:onProperty ; + owl:minQualifiedCardinality "1"^^xsd:nonNegativeInteger ; + owl:onClass + ] , + [ rdf:type owl:Restriction ; + owl:onProperty ; + owl:maxQualifiedCardinality "7"^^xsd:nonNegativeInteger ; + owl:onClass + ] . + + +### Generated by the OWL API (version 4.5.26) https://github.com/owlcs/owlapi From 0de578cc5a4f56d50b294bbf966f6c508811eaaa Mon Sep 17 00:00:00 2001 From: "James A. Overton" Date: Tue, 28 Nov 2023 18:12:57 -0500 Subject: [PATCH 5/8] Try disabling subset integration test --- docs/extract.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/extract.md b/docs/extract.md index da832212d..0ef88d43e 100644 --- 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`. From 54139f8f4835a6bae5b7d071a938f0b12da3c188 Mon Sep 17 00:00:00 2001 From: "James A. Overton" Date: Wed, 29 Nov 2023 08:50:47 -0500 Subject: [PATCH 6/8] Rename addLabels() to addEntities() --- .../java/org/obolibrary/robot/Template.java | 167 +++++++++--------- 1 file changed, 86 insertions(+), 81 deletions(-) diff --git a/robot-core/src/main/java/org/obolibrary/robot/Template.java b/robot-core/src/main/java/org/obolibrary/robot/Template.java index 21aeb8d51..d95ec8135 100644 --- 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> 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> rows, IOHelper // Add the contents of the tableRows addTable(rows); - addLabels(); + addEntities(); createParser(); } @@ -237,7 +237,7 @@ public Template(@Nonnull String name, @Nonnull List> 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,99 +557,104 @@ private void addTable(List> rows) throws Exception { } } - /** Add the labels from the rows of the template to the QuotedEntityChecker. */ - private void addLabels() { + /** Add the entities from the rows of the template to the QuotedEntityChecker. */ + private void addEntities() { for (List row : tableRows) { - String id = null; - try { - id = row.get(idColumn); - } catch (IndexOutOfBoundsException e) { - // ignore - } + addEntity(row); + } + } - if (id == null) { - continue; - } + /** Add the entity from this row of the template to the QuotedEntityChecker. */ + private void addEntity(List row) { + String id = null; + try { + id = row.get(idColumn); + } catch (IndexOutOfBoundsException e) { + // ignore + } + + if (id == null) { + return; + } + + String label = null; + try { + label = row.get(labelColumn); + } catch (IndexOutOfBoundsException e) { + // ignore + } - String label = null; + 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"; + } - 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); - } + if (id != null) { + checker.add(entity, id); + } + if (label != null) { + checker.add(entity, label); } } From 108d0e28e4dcceecda0e319a21d52c20856ba18e Mon Sep 17 00:00:00 2001 From: "James A. Overton" Date: Wed, 29 Nov 2023 08:57:17 -0500 Subject: [PATCH 7/8] Rename test for template without types or labels --- robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java index c3233d4b5..f283ea790 100644 --- a/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java +++ b/robot-core/src/test/java/org/obolibrary/robot/TemplateTest.java @@ -55,7 +55,7 @@ public void testLegacyTemplateCSV() throws Exception { * @throws Exception if entities cannot be found */ @Test - public void testLegacyTemplateCSV3() throws Exception { + public void testNoLabelsNoTypes() throws Exception { String path = "/sequence-template.csv"; List> rows = TemplateHelper.readCSV(this.getClass().getResourceAsStream(path)); Template t = new Template(path, rows); From 3de1c0c93d3b1b558b0f01155ad6c9f5f990fc2f Mon Sep 17 00:00:00 2001 From: "James A. Overton" Date: Wed, 29 Nov 2023 09:08:20 -0500 Subject: [PATCH 8/8] Update CHANGELOG for Template fix --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ad04bdae..c472789b3 100644 --- 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