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

Suggested modifications for OWLGraphEdge to handle OWLAnnotation #81

Closed
ShahimEssaid opened this issue Feb 2, 2015 · 6 comments
Closed

Comments

@ShahimEssaid
Copy link

From frederic...@gmail.com on October 08, 2013 07:42:30

Hi,

to date, OWLAnnotations are not taken into account when instantiating an OWLGraphEdge. This is an outcome when removing an OWLAxiom generated from an OWLGraphEdge, or when checking for its existence. This is because axiom annotations affect the structural identity of an axiom (see 4.2 here: http://webont.org/owled/2009/papers/owled2009_submission_29.pdf).
Example of problematic code provided at the end of this message.

I suggest the following modifications, see diff file attached.

NOTE OF CAUTION: if this patch was applied, OWLGraphEdges with the same "struture", but different annotations, would not be considered equal. For me, this is a bug fix, but you might disagree ;)
As a result, the method OWLGraphWrapperEdges#getPrimitiveOutgoingEdges(OWLObject, Set) would produce more OWLGraphEdges than before:
this is because in this method, OWLEquivalentClassesAxiom are transformed into OWLSubClassOfAxiom. So, when an intersectionof property is annotated, and the equivalent subclassof axiom is present in the ontology, but not annotated, this produces two distinct OWLGraphEdges.

Consider the following example:
[Term]
id: UBERON:0010032
name: anterior part of tongue
...
is_a: UBERON:0000064 ! organ part
relationship: develops_from UBERON:0006260 ! lingual swellings
relationship: part_of UBERON:0001723 ! tongue
...
intersection_of: UBERON:0000064 {source="ISBN10:1607950324"} ! organ part
intersection_of: develops_from UBERON:0006260 {source="ISBN10:1607950324"} ! lingual swellings
intersection_of: part_of UBERON:0001723 {source="ISBN10:1607950324"} ! tongue

Without this patch, you would retrieve 3 OWLGraphEdges for the relations above. With this patch, you would retrieve 6.
In a first test, I added annotation properties to OWLGraphEdge, without taking them into account for equality; but in that case, the annotations could or could not be retrieved, simply depending on the iteration order of the OWLAxioms.


All the tests ran smoothly with this patch applied. Let me know what you think. Thanks!



------------------------
Example of problematic code, this would fail if the OWLAxiom had any annotations:

OWLSubClassOfAxiom ax = getManager().getOWLDataFactory().getOWLSubClassOfAxiom((OWLClassExpression) edge.getSource(), (OWLClassExpression) edgeToTargetExpression(edge));

//the axiom is not found and not removed
getManager().removeAxiom(edge.getOntology(), ax);

It would be necessary to do the following before removing the axiom (if OWLGraphEdge had a getAnnotations method):

if (edge.getAnnotations() != null && !edge.getAnnotations().isEmpty()) {
ax = (OWLSubClassOfAxiom) ax.getAnnotatedAxiom(edge.getAnnotations());
}

Attachment: diff.txt

Original issue: http://code.google.com/p/owltools/issues/detail?id=81

@ShahimEssaid
Copy link
Author

From frederic...@gmail.com on October 08, 2013 11:56:33

Owner: cmung...@gmail.com

@ShahimEssaid
Copy link
Author

From cmung...@gmail.com on October 21, 2013 11:05:16

The patch looks sound.

There may be some performance impact, but should be minimal.

My one concern is that there may be code that tests for the existence of a graph edge - this would fail unless annotations match. This may be counter to expectations if the programmer is assuming a simple graph model.

An alternative would be to have each edge be aware of the list of axioms that support the edge. getAnnotations() would iterate through these and return the union. Here there would be no change in identity conditions for an edge. Edges would only be distinsguished if they were logically different.

Would this work for your use case?

@ShahimEssaid
Copy link
Author

From cmung...@gmail.com on October 21, 2013 19:47:19

This issue was updated by revision r1658 .

@ShahimEssaid
Copy link
Author

From frederic...@gmail.com on October 22, 2013 08:29:32

There is the containsAxiomIgnoreAnnotations and getAxiomsIgnoreAnnotations methods to test for existence.

But I like your idea to avoid getting "duplicated" edges. I could then use methods such as getOWLAxioms and getSubClassOfAxioms, implemented in OWLGraphEdge. I would go for that solution.

(just to mention that I was not interested in getting the annotations in the first place, but in getting the proper underlying axioms)

@ShahimEssaid
Copy link
Author

From frederic...@gmail.com on November 15, 2013 17:02:40

Hi, here are my proposed modifications to implement your suggestion, see diff file attached. Note that this would also fix issue #66 .

The only change that could impact existing code, is that the methods OWLGraphEdge#hashCode() and OWLGraphEdge#equals(Object) now take into account the ontology attribute. Otherwise, these modifications should be invisible from the API point of view. All unit tests passed without failure.

The reason for this change is that, as different ontologies could potentially contain structurally equivalent axioms, a same OWLGraphEdge could store OWLAxioms not applicable to its ontology attribute.
An alternative would be to allow a same OWLGraphEdge to be associated to different ontologies, but that would change the signature of the class a lot... (getOntologies() vs. getOntology(), etc).

So, as a result, two structurally equivalent axioms in 2 different ontologies would generate two OWLGraphEdges, not only one as today. It doesn't sound like a big deal to me.

Let me know what you think! (BTW, it's the first time that I come to understand how the getOutgoingEdges method really work, I feel a bit ashamed, I never noticed that the final OWLGraphEdges returned were produced recursively :/)

Attachment: diff.txt

@ShahimEssaid
Copy link
Author

From frederic...@gmail.com on November 20, 2013 12:41:35

Modifications accepted, see revision r1764 . Issue closed.

Status: Verified

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant