Skip to content

Commit

Permalink
Consistently use 'about_edge' slot.
Browse files Browse the repository at this point in the history 
The KGCL model is somewhat inconsistent in that the EdgeChange class has
a 'about_edge' slot intended to represent the edge (as a {subject,
predicate, object} triple) that is being changed, but several of its
subclasses (EdgeCreation, EdgeDeletion, MoveUnder, PlaceUnder) _also_
have distinct 'subject', 'predicate', and 'object' slots, which are
redundant with the fields of the 'about_edge' slot.

I believe these slots are not only useless but also harmful, since it is
unclear how the corresponding classes are supposed to be used (i.e.,
should the subject be set in change.subject or in
change.about_edge.subject?). Since I strongly suspect these slots are
actually a mistake (and got no indication of the contrary when I raised
the issue to the KGCL folks [1]), here we update our code to only deal
with the 'about_edge' slot, and completely ignore the "flattened"
subject/object/predicate slots.

[1] INCATools/kgcl#53
  • Loading branch information
@gouttegd
gouttegd committed Sep 27, 2024
1 parent d213c8c commit df17e12
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 76 deletions.
17 changes: 9 additions & 8 deletions core/src/main/java/org/incenp/obofoundry/kgcl/KGCLTextTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -314,26 +314,27 @@ public String visit(ObjectPropertyCreation v) {

@Override
public String visit(EdgeCreation v) {
return String.format("create edge %s %s %s", renderNode(v.getSubject()), renderNode(v.getPredicate()),
renderNode(v.getObject()));
return String.format("create edge %s %s %s", renderNode(v.getAboutEdge().getSubject()),
renderNode(v.getAboutEdge().getPredicate()),
renderNode(v.getAboutEdge().getObject()));
}

@Override
public String visit(EdgeDeletion v) {
return String.format("delete edge %s %s %s", renderNode(v.getSubject()), renderNode(v.getPredicate()),
renderNode(v.getObject()));
return String.format("delete edge %s %s %s", renderNode(v.getAboutEdge().getSubject()),
renderNode(v.getAboutEdge().getPredicate()), renderNode(v.getAboutEdge().getObject()));
}

@Override
public String visit(PlaceUnder v) {
return String.format("create edge %s rdfs:subClassOf %s", renderNode(v.getSubject()),
renderNode(v.getObject()));
return String.format("create edge %s rdfs:subClassOf %s", renderNode(v.getAboutEdge().getSubject()),
renderNode(v.getAboutEdge().getObject()));
}

@Override
public String visit(RemoveUnder v) {
return String.format("delete edge %s rdfs:subClassOf %s", renderNode(v.getSubject()),
renderNode(v.getObject()));
return String.format("delete edge %s rdfs:subClassOf %s", renderNode(v.getAboutEdge().getSubject()),
renderNode(v.getAboutEdge().getObject()));
}

@Override
Expand Down
23 changes: 12 additions & 11 deletions core/src/main/java/org/incenp/obofoundry/kgcl/owl/DirectOWLTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -632,17 +632,17 @@ public List<OWLOntologyChange> visit(ObjectPropertyCreation v) {
@Override
public List<OWLOntologyChange> visit(EdgeCreation v) {
// TODO: Support subject and object being something else than OWL classes
IRI subjectIRI = findClass(v, v.getSubject().getId());
IRI subjectIRI = findClass(v, v.getAboutEdge().getSubject().getId());
if ( subjectIRI == null ) {
return empty;
}

IRI predicateIRI = IRI.create(v.getPredicate().getId());
IRI objectIRI = IRI.create(v.getObject().getId());
IRI predicateIRI = IRI.create(v.getAboutEdge().getPredicate().getId());
IRI objectIRI = IRI.create(v.getAboutEdge().getObject().getId());
OWLAxiom edgeAxiom = null;
EdgeType edgeType = getEdgeType(predicateIRI);
if ( edgeType == null ) {
onReject(v, "Edge predicate <%s> not found", v.getPredicate().getId());
onReject(v, "Edge predicate <%s> not found", v.getAboutEdge().getPredicate().getId());
return empty;
}

Expand All @@ -668,9 +668,10 @@ public List<OWLOntologyChange> visit(EdgeCreation v) {

@Override
public List<OWLOntologyChange> visit(EdgeDeletion v) {
IRI subjectIRI = findClass(v, v.getSubject().getId());
IRI objectIRI = findClass(v, v.getObject().getId());
IRI predicateIRI = v.getPredicate() != null ? IRI.create(v.getPredicate().getId()) : null;
IRI subjectIRI = findClass(v, v.getAboutEdge().getSubject().getId());
IRI objectIRI = findClass(v, v.getAboutEdge().getObject().getId());
IRI predicateIRI = v.getAboutEdge().getPredicate() != null ? IRI.create(v.getAboutEdge().getPredicate().getId())
: null;

if ( subjectIRI == null || objectIRI == null ) {
return empty;
Expand All @@ -697,10 +698,10 @@ public List<OWLOntologyChange> visit(PlaceUnder v) {
* explicitly set to rdfs:subClassOf, so in case it has not been set we do so
* here.
*/
if ( v.getPredicate() == null ) {
if ( v.getAboutEdge().getPredicate() == null ) {
Node predicate = new Node();
predicate.setId(OWLRDFVocabulary.RDFS_SUBCLASS_OF.toString());
v.setPredicate(predicate);
v.getAboutEdge().setPredicate(predicate);
}

return visit((EdgeCreation) v);
Expand All @@ -715,10 +716,10 @@ public List<OWLOntologyChange> visit(RemoveUnder v) {
* memory. In this case it's unclear whether the predicate should be explicitly
* set to rdfs:subClassOf, so in case it has not been set we do so here.
*/
if ( v.getPredicate() == null ) {
if ( v.getAboutEdge().getPredicate() == null ) {
Node predicate = new Node();
predicate.setId(OWLRDFVocabulary.RDFS_SUBCLASS_OF.toString());
v.setPredicate(predicate);
v.getAboutEdge().setPredicate(predicate);
}

return visit((EdgeDeletion) v);
Expand Down
12 changes: 7 additions & 5 deletions core/src/main/java/org/incenp/obofoundry/kgcl/owl/ProvisionalOWLTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public List<OWLOntologyChange> visit(NodeObsoletionWithNoDirectReplacement v) {

@Override
public List<OWLOntologyChange> visit(EdgeCreation v) {
IRI nodeIRI = IRI.create(v.getSubject().getId());
IRI nodeIRI = IRI.create(v.getAboutEdge().getSubject().getId());
if ( !ontology.containsEntityInSignature(nodeIRI) ) {
onReject(v, "Node <%s> not found in signature", nodeIRI.toString());
return empty;
Expand All @@ -135,8 +135,10 @@ public List<OWLOntologyChange> visit(EdgeCreation v) {
ArrayList<OWLOntologyChange> changes = new ArrayList<OWLOntologyChange>();
HashSet<OWLAnnotation> annots = new HashSet<OWLAnnotation>();

annots.add(factory.getOWLAnnotation(getKGCLProperty("predicate"), IRI.create(v.getPredicate().getId())));
annots.add(factory.getOWLAnnotation(getKGCLProperty("object"), IRI.create(v.getObject().getId())));
annots.add(factory.getOWLAnnotation(getKGCLProperty("predicate"),
IRI.create(v.getAboutEdge().getPredicate().getId())));
annots.add(
factory.getOWLAnnotation(getKGCLProperty("object"), IRI.create(v.getAboutEdge().getObject().getId())));
addMetadata(v, annots);

changes.add(new AddAxiom(ontology, factory.getOWLAnnotationAssertionAxiom(pendingChangeProperty, nodeIRI,
Expand All @@ -147,10 +149,10 @@ public List<OWLOntologyChange> visit(EdgeCreation v) {

@Override
public List<OWLOntologyChange> visit(PlaceUnder v) {
if ( v.getPredicate() == null ) {
if ( v.getAboutEdge().getPredicate() == null ) {
Node predicate = new Node();
predicate.setId(OWLRDFVocabulary.RDFS_SUBCLASS_OF.toString());
v.setPredicate(predicate);
v.getAboutEdge().setPredicate(predicate);
}

return visit((EdgeCreation) v);
Expand Down
16 changes: 10 additions & 6 deletions core/src/main/java/org/incenp/obofoundry/kgcl/parser/ParseTree2ChangeVisitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,9 +285,11 @@ public Void visitNewNode(KGCLParser.NewNodeContext ctx) {
@Override
public Void visitNewEdge(KGCLParser.NewEdgeContext ctx) {
EdgeCreation change = new EdgeCreation();
change.setSubject(getNode(ctx.subject_id));
change.setPredicate(getNode(ctx.predicate_id));
change.setObject(getNode(ctx.object_id));
Edge edge = new Edge();
edge.setSubject(getNode(ctx.subject_id));
edge.setPredicate(getNode(ctx.predicate_id));
edge.setObject(getNode(ctx.object_id));
change.setAboutEdge(edge);
maybeAddChange(change);

return null;
Expand All @@ -296,9 +298,11 @@ public Void visitNewEdge(KGCLParser.NewEdgeContext ctx) {
@Override
public Void visitDeleteEdge(KGCLParser.DeleteEdgeContext ctx) {
EdgeDeletion change = new EdgeDeletion();
change.setSubject(getNode(ctx.subject_id));
change.setPredicate(getNode(ctx.predicate_id));
change.setObject(getNode(ctx.object_id));
Edge edge = new Edge();
edge.setSubject(getNode(ctx.subject_id));
edge.setPredicate(getNode(ctx.predicate_id));
edge.setObject(getNode(ctx.object_id));
change.setAboutEdge(edge);
maybeAddChange(change);

return null;
Expand Down
26 changes: 15 additions & 11 deletions core/src/test/java/org/incenp/obofoundry/kgcl/KGCLReaderTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import org.incenp.obofoundry.kgcl.model.AddNodeToSubset;
import org.incenp.obofoundry.kgcl.model.Change;
import org.incenp.obofoundry.kgcl.model.ClassCreation;
import org.incenp.obofoundry.kgcl.model.Edge;
import org.incenp.obofoundry.kgcl.model.EdgeCreation;
import org.incenp.obofoundry.kgcl.model.EdgeDeletion;
import org.incenp.obofoundry.kgcl.model.NewSynonym;
Expand Down Expand Up @@ -94,9 +95,10 @@ void testFileParser() throws IOException {
c4.setNewValue("xyz");

EdgeCreation c5 = new EdgeCreation();
c5.setSubject(util.getNode("0004"));
c5.setPredicate(util.getForeignNode(OWLRDFVocabulary.RDFS_SUBCLASS_OF.toString()));
c5.setObject(util.getNode("0003"));
c5.setAboutEdge(new Edge());
c5.getAboutEdge().setSubject(util.getNode("0004"));
c5.getAboutEdge().setPredicate(util.getForeignNode(OWLRDFVocabulary.RDFS_SUBCLASS_OF.toString()));
c5.getAboutEdge().setObject(util.getNode("0003"));

Change[] expected = { c1, c2, c3, c4, c5 };
for (int i = 0; i < 5; i++) {
Expand Down Expand Up @@ -332,8 +334,8 @@ void testCreateNodesWithoutExplicitIds() {

Assertions.assertInstanceOf(EdgeCreation.class, changeset.get(2));
EdgeCreation c3 = EdgeCreation.class.cast(changeset.get(2));
Assertions.assertEquals(c1.getAboutNode().getId(), c3.getSubject().getId());
Assertions.assertEquals(c2.getAboutNode().getId(), c3.getObject().getId());
Assertions.assertEquals(c1.getAboutNode().getId(), c3.getAboutEdge().getSubject().getId());
Assertions.assertEquals(c2.getAboutNode().getId(), c3.getAboutEdge().getObject().getId());
}

@Test
Expand Down Expand Up @@ -490,19 +492,21 @@ void testNewAnnotationPropertyChange() {
@Test
void testNewEdgeChange() {
EdgeCreation change = new EdgeCreation();
change.setSubject(util.getNode("0001"));
change.setPredicate(util.getNode("is_related_to"));
change.setObject(util.getNode("0002"));
change.setAboutEdge(new Edge());
change.getAboutEdge().setSubject(util.getNode("0001"));
change.getAboutEdge().setPredicate(util.getNode("is_related_to"));
change.getAboutEdge().setObject(util.getNode("0002"));

testParse("create edge EX:0001 EX:is_related_to EX:0002", change);
}

@Test
void testDeleteEdgeChange() {
EdgeDeletion change = new EdgeDeletion();
change.setSubject(util.getNode("0001"));
change.setPredicate(util.getNode("is_related_to"));
change.setObject(util.getNode("0002"));
change.setAboutEdge(new Edge());
change.getAboutEdge().setSubject(util.getNode("0001"));
change.getAboutEdge().setPredicate(util.getNode("is_related_to"));
change.getAboutEdge().setObject(util.getNode("0002"));

testParse("delete edge EX:0001 EX:is_related_to EX:0002", change);
}
Expand Down
25 changes: 15 additions & 10 deletions core/src/test/java/org/incenp/obofoundry/kgcl/KGCLTextTranslatorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.incenp.obofoundry.kgcl.model.AddNodeToSubset;
import org.incenp.obofoundry.kgcl.model.Change;
import org.incenp.obofoundry.kgcl.model.ClassCreation;
import org.incenp.obofoundry.kgcl.model.Edge;
import org.incenp.obofoundry.kgcl.model.EdgeCreation;
import org.incenp.obofoundry.kgcl.model.EdgeDeletion;
import org.incenp.obofoundry.kgcl.model.NewSynonym;
Expand Down Expand Up @@ -225,37 +226,41 @@ void testRenderAnnotationPropertyCreation() {
@Test
void testRenderEdgeCreation() {
EdgeCreation change = new EdgeCreation();
change.setSubject(util.getNode("0001"));
change.setPredicate(util.getNode("is_related_to"));
change.setObject(util.getNode("0002"));
change.setAboutEdge(new Edge());
change.getAboutEdge().setSubject(util.getNode("0001"));
change.getAboutEdge().setPredicate(util.getNode("is_related_to"));
change.getAboutEdge().setObject(util.getNode("0002"));

render(change, "create edge EX:0001 EX:is_related_to EX:0002");
}

@Test
void testRenderEdgeDeletion() {
EdgeDeletion change = new EdgeDeletion();
change.setSubject(util.getNode("0001"));
change.setPredicate(util.getNode("is_related_to"));
change.setObject(util.getNode("0002"));
change.setAboutEdge(new Edge());
change.getAboutEdge().setSubject(util.getNode("0001"));
change.getAboutEdge().setPredicate(util.getNode("is_related_to"));
change.getAboutEdge().setObject(util.getNode("0002"));

render(change, "delete edge EX:0001 EX:is_related_to EX:0002");
}

@Test
void testRendePlaceUnder() {
PlaceUnder change = new PlaceUnder();
change.setSubject(util.getNode("0001"));
change.setObject(util.getNode("0002"));
change.setAboutEdge(new Edge());
change.getAboutEdge().setSubject(util.getNode("0001"));
change.getAboutEdge().setObject(util.getNode("0002"));

render(change, "create edge EX:0001 rdfs:subClassOf EX:0002");
}

@Test
void testRenderRemoveUnder() {
RemoveUnder change = new RemoveUnder();
change.setSubject(util.getNode("0001"));
change.setObject(util.getNode("0002"));
change.setAboutEdge(new Edge());
change.getAboutEdge().setSubject(util.getNode("0001"));
change.getAboutEdge().setObject(util.getNode("0002"));

render(change, "delete edge EX:0001 rdfs:subClassOf EX:0002");
}
Expand Down
Loading

0 comments on commit df17e12

Please sign in to comment.