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

Updating pro import #2413

Merged
merged 2 commits into from
Apr 12, 2022
Merged

Updating pro import #2413

merged 2 commits into from
Apr 12, 2022

Conversation

shawntanzk
Copy link
Collaborator

Fixes #2373
@matentzn - doesnt seem to fix after updating pro -> not sure if it got updated right? (I update mirror and all) could you help me check if the diff has the changes?

Fixes #2373
@matentzn - doesnt seem to fix after updating pro -> not sure if it got updated right? (I update mirror and all) could you help me check if the diff has the changes?
@shawntanzk shawntanzk requested a review from matentzn April 11, 2022 21:03
@shawntanzk shawntanzk self-assigned this Apr 11, 2022
Copy link
Contributor

@matentzn matentzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to solve the issue.. Hmmmm.. How did no one ever notice that?

@matentzn
Copy link
Contributor

I provided an ODK fix..

We should prioritise moving Uberon to base module system..

@shawntanzk
Copy link
Collaborator Author

@matentzn the PRO-short-label is still there in this branch

<!-- http://purl.obolibrary.org/obo/pr#PRO-common-name -->

<owl:AnnotationProperty rdf:about="http://purl.obolibrary.org/obo/pr#PRO-common-name">
<rdfs:subPropertyOf rdf:resource="http://www.geneontology.org/formats/oboInOwl#SynonymTypeProperty"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key row @shawntanzk

If you run sh run.sh make uberon.obo IMP=false PAT=false BRI=false the subsets should look correct.

@matentzn
Copy link
Contributor

@shawntanzk How did you verify this? I checked the source, and it looks correct (I also tagged you on the line so you can see it)

@shawntanzk
Copy link
Collaborator Author

@shawntanzk How did you verify this? I checked the source, and it looks correct (I also tagged you on the line so you can see it)

Oh I thought you wanted to remove it, I just checked in protege if it was still there. But I see that you changed it to a has_synonym_type AP

@matentzn
Copy link
Contributor

I dont know whether I ever explained the problem to you: Basically OBO format has this thing where it expects synonym types and subsets to be sub-annotation-properties of some specific parent. But the ROBOT module extraction process does not know about that - and robot extract drops this connection (the parentage). When this happens, OBO format does not know what to do anymore. Thats why we need to inject these sub-properties back in with SPARQL postprocessing. IIIIII HACK

@shawntanzk
Copy link
Collaborator Author

btws is this ready to merge? also do we have to do a release for this?

@matentzn
Copy link
Contributor

Yeah, lets do another release on Friday. It will be easier now.

@matentzn
Copy link
Contributor

Sure, merge

@shawntanzk shawntanzk merged commit d84344e into master Apr 12, 2022
@shawntanzk shawntanzk deleted the issue2373-update-pro branch April 12, 2022 10:38
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.

OBO file uses undeclared synonym type def
2 participants