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

Potential overwriting of attributes in imported modules #1118

Closed
markus-rothkoetter opened this issue Apr 26, 2022 · 12 comments
Closed

Potential overwriting of attributes in imported modules #1118

markus-rothkoetter opened this issue Apr 26, 2022 · 12 comments
Labels
bug Something isn't working external ontology external ontologies that could be included ontology infrastructure

Comments

@markus-rothkoetter
Copy link
Contributor

Description of the issue

Changes introduced in #1086 (e.g. adding Inverse Of) reside in ro-module which is an imported module.
The content of the ro-module is automatically extracted via ROBOT 🤖.

Inspecting the extract-ro-module.sh-script, I have NOT seen any precaution checking for potential changes BEFORE recreating the ro-module from scratch/source.

From my perspective this means: As soon as the script is run, all previous edits are lost immediately.

Can someone comment whether this is true or if there any taken measures I'm not aware of?
@jannahastings, @stap-m, @MGlauer

Ideas of solution

In case I'm right about my assumption, I see the following options:

  1. implement a diff check in the script and merge changes
  2. change the location of implementation of attributes in imported modules
@markus-rothkoetter markus-rothkoetter added question Further information is requested external ontology external ontologies that could be included To do Issues that haven't got discussed yet ontology infrastructure labels Apr 26, 2022
@jannahastings
Copy link
Contributor

This is indeed true, and I think your option 1. is the better one. We did not so far re-run this script after initially creating the module, but there may be situations in the future where we would need to do so.

@markus-rothkoetter markus-rothkoetter added bug Something isn't working and removed question Further information is requested labels Apr 26, 2022
@markus-rothkoetter
Copy link
Contributor Author

As this is the bottleneck for #1114 & #1023 (both require additional ro-relations), I created a simple diff/merging command in the script and created the ro-module from scratch. (mentioned as Option 1 before)

TL;DR: @stap-m @l-emele @jannahastings
The diff-check showed some issues (caused by past manual edits) which break a simple recreation of the file (see next comment)
With respect to the upcoming release:
I can merge the terms mentioned in #1023 manually via ROBOT into the existing ro-module and treat #1114 & #1023 afterwards. This means the script fix has to wait until the release is done.

Is this a valid approach?

@markus-rothkoetter
Copy link
Contributor Author


BFO_0000019 aka quality resides as a class in ro-module
(This is introduced by the annotation property is homeomorphic for)
🔑-question no. 1: Shall this be moved to the defining-ontology of quality?

BFO_0000040 aka material entity is pulled into the module by RO_0002577 aka system.
I found the commit where @jannahastings added this to the ontology and another commit removing major parts of BFO_0000040 class definition. (I assume in order to remove duplication?)
🔑-question no. 2: I assume this shall remain, so I'm going to add this to the extract-no-hierarchy.txt?

RO_0000086 aka has quality is in the ontology (including subclasses), but not in the extract-lists for the script.
Added during commit
🔑-question no. 3: Shall I add this to the extract.txt?

IAO_0000115 aka definition, an annotation property, resides in ro-module
🔑-question no. 4: I think it belongs into the iao-module. Shall I move it there?

oboInOwl:inSubset is now added to multiple properties, when recreating the ro-module
🔑-question no. 5 : @jannahastings Is this needed or overhead? Shall we excluded it via ROBOT?

foaf:page is an annotation property used once and there is also an import related to foaf.
I don't know what this does and how to treat it in the import script as the from-scratch version does not contain it anymore.
🔑-question no. 6: What to do? :)

Lastly, there are some definition updates different from the current oeo-imported-version e.g.:

  • has output:
    "p has output ... and c is not present at the beginning of p."
    vs
    "p has output ... and c is not present in the same state at the beginning of p."
  • has characteristic:
    "a relation between an independent continuant (the bearer) and a specifically dependent continuant (the dependent), in which the dependent specifically depends on the bearer for its existence"
    vs
    "Inverse of characteristic_of"

🔑-question no. 7: Shall the updated ro-definition be used or is there a different approach?

@github-actions github-actions bot removed the To do Issues that haven't got discussed yet label May 3, 2022
@jannahastings
Copy link
Contributor

BFO_0000019 aka quality resides as a class in ro-module
(This is introduced by the annotation property is homeomorphic for)
🔑-question no. 1: Shall this be moved to the defining-ontology of quality?

Yes, `quality' should be a class in BFO. Isn't it already there?

@jannahastings
Copy link
Contributor

BFO_0000040 aka material entity is pulled into the module by RO_0002577 aka system.
I found the commit where @jannahastings added this to the ontology and another commit removing major parts of BFO_0000040 class definition. (I assume in order to remove duplication?)
🔑-question no. 2: I assume this shall remain, so I'm going to add this to the extract-no-hierarchy.txt?

I don't remember why I did this but yes we don't want to duplicate any annotations so if we include the declaration it should be bare without additional axioms.

@jannahastings
Copy link
Contributor

RO_0000086 aka has quality is in the ontology (including subclasses), but not in the extract-lists for the script.
Added during commit
🔑-question no. 3: Shall I add this to the extract.txt?

No objections, sounds sensible to me

@jannahastings
Copy link
Contributor

IAO_0000115 aka definition, an annotation property, resides in ro-module
🔑-question no. 4: I think it belongs into the iao-module. Shall I move it there?

Yes (isn't it already there?) but in the medium term we need to move to using the 'obo metadata ontology' which is a new and updated version of the IAO ontology metadata tags such as these, which I think is the new primary source for commonly used shared annotation properties.

@jannahastings
Copy link
Contributor

jannahastings commented May 4, 2022

oboInOwl:inSubset is now added to multiple properties, when recreating the ro-module
🔑-question no. 5 : @jannahastings Is this needed or overhead? Shall we excluded it via ROBOT?

Indeed I don't think we need this. Although it might potentially be a good candidate for the annotation property which flags which module a particular class is defined in, see #870

@jannahastings
Copy link
Contributor

foaf:page is an annotation property used once and there is also an import related to foaf.
I don't know what this does and how to treat it in the import script as the from-scratch version does not contain it anymore.
🔑-question no. 6: What to do? :)

Ugh can we remove this ?

@jannahastings
Copy link
Contributor

🔑-question no. 7: Shall the updated ro-definition be used or is there a different approach?

The updates from RO look good to me.

@markus-rothkoetter
Copy link
Contributor Author

markus-rothkoetter commented May 5, 2022

No action required currently 🚧, just a note for our future updates to the script:
Ontology IRI and Version IRI are different in the current ro-module-version from the script-generated one.

script

 --ontology-iri http://open-energy-ontology.org/ontology/imports/ro-module.owl 
 --version-iri http://open-energy-ontology.org/ontology/imports/ro-module.owl 

vs

current module

 --ontology-iri http://openenergy-platform.org/ontology/oeo/imports/ro-module.owl 
 --version-iri http://openenergy-platform.org/ontology/oeo/dev/imports/ro-module.owl 

@markus-rothkoetter
Copy link
Contributor Author

Discussions are moved to follow-up issue #1154
(This topic touches also other areas not covered in this issue.
So, information are collected in one place (#1154) for reworking the import process)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external ontology external ontologies that could be included ontology infrastructure
Projects
None yet
Development

No branches or pull requests

3 participants