search

INCATools / ontology-development-kit

Bootstrap an OBO Library ontology
http://incatools.github.io/ontology-development-kit/
BSD 3-Clause "New" or "Revised" License
212 stars 53 forks source link

v1.5 Stripping excess annotation properties from imports is removing annotated RO role chains #1024

Open Clare72 opened 4 months ago

Clare72 commented 4 months ago

for example - from RO:

[Typedef] id: RO:0002216 name: capable of part of def: "c stands in this relationship to p if and only if there exists some p' such that c is capable_of p', and p' is part_of p." [] property_value: IAO:0000117 https://orcid.org/0000-0002-6601-2165 property_value: IAO:0000118 "has function in" xsd:string property_value: seeAlso http://purl.obolibrary.org/obo/ro/docs/reflexivity/ holds_over_chain: RO:0002215 BFO:0000050 {RO:0002582="true"} is_a: RO:0002328 ! functionally related to is_a: RO:0002500 ! causal agent in process

Default annotation properties to keep seem to be just label and definition (I can see why this might be desirable), but this causes the annotated role chain to be stripped out. This causes some loss of inference in FBbt.

I can fix this by amending my -odk.yaml file to have the following under import_group, but I think logical axioms should probably not be stripped out of imports at all.

annotation_properties:

  • RO:0002582
  • rdfs:label
  • IAO:0000115
gouttegd commented 4 months ago

@matentzn I wonder if it could be a ROBOT bug.

Considering the following minimal example ontology:

Prefix(:=<http://example.org/odk1024.owl#>)

Ontology(<http://example.org/odk1024.owl>
<http://example.org/odk1024.owl>

Declaration(AnnotationProperty(:AP1))
Declaration(ObjectProperty(:OP1))
Declaration(ObjectProperty(:OP2))

SubObjectPropertyOf(Annotation(:AP1 "Annotation on a SubObjectPropertyOf axiom") :OP2 :OP1)
SubObjectPropertyOf(Annotation(:AP1 "Annotation on a SubObjectPropertyOf axiom with a ObjectPropertyChain operand") ObjectPropertyChain(:OP2 :OP1) :OP2)

)

Asking ROBOT to remove the :AP1 annotation property (remove --term http://example.org/odk1024.owl#AP1 --select annotation-properties) results in the following changes:

This is what was happening in FBbt’s RO import, where the following axiom:

SubObjectPropertyOf(Annotation(<http://purl.obolibrary.org/obo/RO_0002582> "true"^^xsd:boolean) ObjectPropertyChain(<http://purl.obolibrary.org/obo/RO_0000052> <http://purl.obolibrary.org/obo/BFO_0000050>) <http://purl.obolibrary.org/obo/RO_0002314>)

was entirely removed because of the removal of the RO:0002582 annotation, where I would have expected an unannotated version of the axiom to be preserved.

matentzn commented 4 months ago

This is a ROBOT bug I think, but the correct behaviour is unfortunately a bit different. We have this parameter --signature true in ROBOT, which supposedly determines if the axiom annotations should be taking into account during remove. You can move the issue there, and keep it as a "inconsistent behaviour" bug in the description, and I will describe how the behaviour should be according to the design.

gouttegd commented 4 months ago

OK, I’ll open a ticket on ROBOT.

In the meantime, I think the ODK should either allow individual ontologies to fallback to the previous behaviour (not stripping annotations from imports at all), or at the very least preserve more annotations than just rdfs:label and IAO:0000115 by default.

Discarding annotations like RO:0002582 by default as if they were somehow “un-important” seems wrong. According to RO’s documentation:

When this design pattern is used, the property chain axiom should be tagged with an is a defining property chain axiom where second argument is reflexive (RO:0002582) annotation.

It seems clear to me that the annotation is part of the design pattern, the ODK should not mess with it.

In fact, arguably all annotations below logical macro assertion (RO:0002416) should be preserved by default, because they may be formally “annotations“ but they are intended to have a logical meaning.

Aside:

We have this parameter --signature true in ROBOT, which supposedly determines if the axiom annotations should be taking into account during remove.

If that is the role of the --signature option, ROBOT’s documentation does a poor job of explaining it.

matentzn commented 4 months ago

It seems clear to me that the annotation is part of the design pattern, the ODK should not mess with it.

I dont know.. I personally do not agree with the fact that these annotations are more than just metadata. I know of no tool and no pipeline that will take these annotations into account. From where I stand, these annotations are interesting for tool developers, but have no meaningful consequence for most users. Unless there is a tangible difference between the presence / absence of these annotations for the average ODK user, I would err on the side of exclusion.

Clare72 commented 4 months ago

I think removing annotation axioms from imports should probably have been an 'opt in' configuration, rather than users now having to specify every annotation they want to keep. It does seem a bit presumptuous to just decide that nobody wants this information and to just start removing it all by default. That said, I do quite like the tidiness of just having label and definition, we certainly do not do anything special with the 'logical macro assertion' annotations (in FlyBase or VFB) and I would not have complained if there had been no changes to the logical axioms.

matentzn commented 4 months ago

It does seem a bit presumptuous to just decide that nobody wants this information and to just start removing it all by default

This is a good point of course. My reasoning behind this was that (1) I kept having to manually remove unnecessary annotations because of confusion and file size limitations and (2) you could sort of say that this is basically doing what robot extract should be doing in the first place: given a set of terms, extract all the information about it (Module = extract(SEED, Ontology)). In my view, this SEED set should include not only the classes, but also the relationships (object and annotation) - this just happens not to be the case (because of the way extract is implemented..