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

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

Open
Clare72 opened this issue Mar 4, 2024 · 6 comments

Comments

@Clare72
Copy link
Contributor

Clare72 commented Mar 4, 2024

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
Copy link
Contributor

gouttegd commented Mar 4, 2024

@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:

  • the SubObjectPropertyOf(Annotation(:AP1 "Annotation on a SubObjectPropertyOf axiom") :OP2 :OP1) axiom is replaced by SubObjectPropertyOf(:OP2 :OP1), as expected (the annotation on the axiom is removed, but the axiom itself is kept);
  • the SubObjectPropertyOf(Annotation(:AP1 "Annotation on a SubObjectPropertyOf axiom with a ObjectPropertyChain operand") ObjectPropertyChain(:OP2 :OP1) :OP2) axiom is completely deleted, which is not what I would have expected (I would have expected it to be replaced by SubObjectPropertyOf(ObjectPropertyChain(:OP2 :OP1) :OP2), i.e. the same axiom without the annotation.

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
Copy link
Contributor

matentzn commented Mar 6, 2024

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
Copy link
Contributor

gouttegd commented Mar 6, 2024

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
Copy link
Contributor

matentzn commented Mar 7, 2024

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
Copy link
Contributor Author

Clare72 commented Mar 7, 2024

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
Copy link
Contributor

matentzn commented Mar 7, 2024

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..

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

No branches or pull requests

3 participants