Rename OMIM axiom annotation properties #698
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,8 +15,8 @@ http://www.geneontology.org/formats/oboInOwl#source | |
| http://www.geneontology.org/formats/oboInOwl#hasSynonymType | ||
| http://www.geneontology.org/formats/oboInOwl#SynonymTypeProperty | ||
| http://purl.obolibrary.org/obo/mondo#GENERATED | ||
| http://purl.obolibrary.org/obo/mondo#includedEntryInOMIM | ||
|
There was a problem hiding this comment. Spoke with Nico today: conclusions.
There was a problem hiding this comment. No, includedEntryInOMIM has nothing to do here; it is not a property. It is a source code value. Yes. It should be kept in mondo. No, it should not be kept in this list. There was a problem hiding this comment. Yes you're right. Not sure why I put that checkbox on this particular PR. But it does also need:
There was a problem hiding this comment. Changes made. This will be ready to be reviewed when the following PR is approved, which changes OMIM_FORMERLY and OMIM_INCLUDED to synonym types: |
||
| http://purl.obolibrary.org/obo/mondo#omimFormerly | ||
| http://www.w3.org/2004/02/skos/core#broadMatch | ||
| http://www.w3.org/2004/02/skos/core#narrowMatch | ||
| http://www.w3.org/2004/02/skos/core#relatedMatch | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Don't these need to actually appear in
properties.txt?During the tech meeting today, @matentzn was surprised I was calling them properties. But I feel like they belong here, alongside
http://purl.obolibrary.org/obo/mondo#GENERATEDwhich I think was disappearing if we didn't have it showing in this file.I could do a before/after test to check this myself but for now I'll just pose the question.
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 dont think this needs to be here. Most of the things you see here, like "GENERATED" are synonym type properties.. That is very different from the
MONDO:includedEntryInOMIMsource annotations (see Mondo file)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.
Aren't the also supposed to be synonym type properties? Is there anything about the nature of
MONDO:GENERATEDthat makes it more appropriate to say that it is a property of the synonym's type than the OMIM ones?I remember that we have been discussing making object property declarations (I believe for these) in
omim.ttl, but have not yet.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.
At some point things were failing without these being added, but this may not be the location that fixed things.
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.
Look at how
includedInOMIMis used in Mondo:As a
oboInOwl:sourceannotation. This is not at all like:Which is how the GENERATED synonym type was used.
The confusion of why
includedEntryInOMIMisn't used the same way, i.e.That question is valid! I don't know the exact answer, but lets say for the sake of rapid progress now that the reason is historical.
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.
Yea maybe.. I dont think you really need the synonym type "omim included", but you may need the synonym type "omim formerly". Right now I think we should do whatever can be done right now and open issues to revise the decision if need be..
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.
@joeflack4 have you looked further into this? Also, I set this to merge into
developsince it was set incorrectly.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 had this on our recent 1:1 agenda, section titled "MONDO: omimFormerly & includedEntryInOMIM".
Can wait until our next meeting if you like.
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.
@joeflack4 can you sort out if this change is needed, try to run relevant make goals with and without the change to check and any other ways you may think of to test this.
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.
@twhetzel May want to just move this to our meeting, that might be easier. This is basically a sub issue of:
omimFormerly,omimIncluded, &includedEntryInOMIM#715And the answers to those questions / decisions will impact whether this is needed or not. If they will become synonym types as Nico seems to suggest might be appropriate, these will definitely be needed.If you / we decide that they should not be changed to that, then I can go ahead and run such an examination now.