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

import from stato #21

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

import from stato #21

wants to merge 7 commits into from

Conversation

LillyG901
Copy link

@LillyG901 LillyG901 commented Nov 10, 2024

Summary of the discussion

See issue #7 and OpenEnergyPlatform/ontology#1133

Type of change (CHANGELOG.md)

Add

Workflow checklist

Automation

Closes #7

PR-Assignee

Reviewer

  • [] 🐙 Follow the Reviewer Guide
  • [] 🐙 Provided feedback and show sufficient appreciation for the work done

@LillyG901 LillyG901 self-assigned this Nov 10, 2024
@LillyG901 LillyG901 closed this Nov 10, 2024
@LillyG901 LillyG901 reopened this Nov 10, 2024
@LillyG901 LillyG901 changed the title Add shell script and list of terms to import from stato import from stato Nov 10, 2024
@LillyG901 LillyG901 marked this pull request as draft November 10, 2024 17:55
@stap-m
Copy link
Contributor

stap-m commented Nov 19, 2024

@LillyG901 I put some more information into the header of the PR.

What about confidence interval, which is also mentioned in the original issue?

@LillyG901
Copy link
Author

LillyG901 commented Nov 19, 2024

Alright, thank you.

Since lower/upper confidence limit is a subclass of confidence interval and we import it with hierarchy confidence interval should be imported alongside the other parent classes.

I will check that this is the case, when I get to test the shell script.

the previous upper term leads to import of more terms than necessary
@LillyG901
Copy link
Author

LillyG901 commented Nov 24, 2024

I misinterpreted the hierarchy on the website I was looking at.
The terms were not related through being in a SubClass relationship, but rather a part of relationship.
That's why it didn't work as intended and I readded the term to the file.

@LillyG901 LillyG901 marked this pull request as ready for review November 24, 2024 14:00
@stap-m
Copy link
Contributor

stap-m commented Nov 27, 2024

grafik

cutoff and statistic have to have their parent classes imported, too, since they are not known by oeo.

@stap-m
Copy link
Contributor

stap-m commented Nov 27, 2024

Can you avoid importing all information of annotation properties? Most of the annotation properties are known by oeo. If we import them again, we have them twice (we had that in the past). Mentioning the IRI should be sufficient. But I am not sure how to achieve that automatically. Maybe you can find that out by reading the ROBOT documentation.

@LillyG901
Copy link
Author

LillyG901 commented Nov 27, 2024

cutoff and statistic have to have their parent classes imported, too, since they are not known by oeo.

I assumed duplicates might cause problems, since we did not import e.g. the BFO parent classes of the iao classes that were imported.
I will fix this.

Can you avoid importing all information of annotation properties? Most of the annotation properties are known by oeo. If we import them again, we have them twice (we had that in the past). Mentioning the IRI should be sufficient. But I am not sure how to achieve that automatically. Maybe you can find that out by reading the ROBOT documentation.

I will look into it.

@stap-m
Copy link
Contributor

stap-m commented Nov 29, 2024

cutoff and statistic have to have their parent classes imported, too, since they are not known by oeo.

I assumed duplicates might cause problems, since we did not import e.g. the BFO parent classes of the iao classes that were imported. I will fix this.

Yes, you're right. You don't have to change that, sorry. We'll have to add the SubClassOf axiom in oeo-import-edits.owl

@LillyG901
Copy link
Author

Alright after some research I found a way to remove the definitions of the annotation properties. If you open the stato-extracted file in Protege, you should now be able to see the IRI of the annotation property instead of the label and the term is no longer defined if you check the Annotation-Property tab.
I'm not sure, if this was the outcome you intended.
I can't remove them further, because I assume we want to keep the information they provide (e.g. the definitions of the terms).

I checked the other import files and we have never really done this before, so it would be great if someone could test this and make sure this is what I should be doing.

@stap-m
Copy link
Contributor

stap-m commented Dec 2, 2024

This looks good now, thanks.
Have you tested an import into oeo-shared?

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

Successfully merging this pull request may close these issues.

import from STATO
2 participants