-
Notifications
You must be signed in to change notification settings - Fork 7
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
Use OMO for synonym type predicates #88
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't run the code, but it looks good.
This is a breaking change for some of my workflows, which filter on the CURIE of the synonym type. The notes for the next release should be clear about the possible breakage. It might also be good to announce this release on OBO Discuss (which we don't bother doing for most releases), or mention it in any announcements about the large OMO synonym migration.
I think that's a good idea @jamesaoverton |
I can make that announcement if you'd like. I would also include some more context about how we are working towards standardizing synoynms across other ontologies too, and similar things might pop up in the near future. Edit: see the announcement |
hi @anitacaron, is there anything else you'd like me to take care of to finish this PR? The OMO definitions now have the correct labels like in OMO:0003000 a owl:AnnotationProperty ;
rdfs:label "abbreviation"^^xsd:string ;
oboInOwl:hasScope "oio:hasBroadSynonym"^^xsd:string ;
rdfs:subPropertyOf oboInOwl:SynonymTypeProperty . |
I'm still not sure about this axiom that is added to the OMO:0003000: |
@anitacaron I also saw that and think there's something fishy, but I didn't want to make any changes to the existing functionality. I'm happy to make a second PR that makes this a proper reference to a IRI instead of a string if you think that will make assessing this PR easier |
This axiom is added to the other annotations, original from NCBITaxon. Now, adding OMO as annotations should not include this. |
So for OMO definitions it shouldn't have the scope added? |
I think so. Someone else could confirm. |
I removed the scope statement from OMO terms in 7f9fb51 |
@anitacaron would you please merge this? I think we're ready for a release, too. |
Closes #87
This PR updates the RDF generation to use OMO terms, when available. It extends the
predicates
configuration dictionary at the top so that new OMO terms can be added in as they are minted. Currently, this PR switches 4 terms to using OMO:There will have to be more detailed follow-up discussion if we want to complete the coverage of OMO on these synonyms.