-
Notifications
You must be signed in to change notification settings - Fork 6
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
Update zfa-edit.obo #166
Update zfa-edit.obo #166
Conversation
Yes, this is right! But, it looks like you added two roots here where only one is corresponding to this ontology. Why did you include the ZFS one? |
@cthoyt when I looked here: https://www.ebi.ac.uk/ols/ontologies/zfa it looks like there are 2 roots in the ontology. Also, the original github issue listed zfa and zfs as needing to be updated in this repo but I didn't see a zfs-edit.obo file so I assumed zfa-edit.obo was the one to update for both. Should I remove the zfs line? |
@erik-whiting that's a really interesting question - I think it's better to keep it focused on terms in the ontology itself, and not ones that are imported. If people want to browse ZFS terms, then I think it makes more sense to start with https://www.ebi.ac.uk/ols/ontologies/zfs. The reason they're both showing up on OLS now is that neither are aligned with an upper level ontology at the moment. If they were, we'd see first |
68082aa
to
c94d8b1
Compare
Updated to only change ZFA |
@cerivs can you please review this? @erik-whiting if we're not able to get in touch via github, you can use the contact information listed on OBO Foundry: https://github.com/OBOFoundry/OBOFoundry.github.io/blob/bfc8cd9756461ba92d24e6f076db4c4af3bc7043/ontology/zfa.md?plain=1#L10-L14 |
It's an odd special case. I think having the ZFS root is probably fine, because in reality ZFS is Managed inside of ZFA and is extracted only after the fact.. |
🤯 then it makes sense to keep both |
c94d8b1
to
5a00b88
Compare
updated to include both ZFA and ZFS |
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.
@ybradford this is safe to merge.
@matentzn sorry to be dense but why do we need to change our root. looking at other AOs - root is not BFO. |
Yeah this is not changing the root, it's just making a statement: these 2 are the root classes of ZFA. This will help us in our grand plan to integrate with COB - it is not changing anything at all in the classification! |
I incorporated this in #168 to be correct. |
Related to OBOFoundry/OBOFoundry.github.io#2149
What this Does
Applies the annotation property has ontology root term (IAO:0000700) to ZFA:0100000
Why this is helpful
the Ontology Lookup Service uses this to help better display ontologies
if the term mentioned above is ever aligned with an upper ontology like BFO, it still will be the thing shown on OLS as the "root"
this will be generally useful for things like alignment with COB since it makes it easier to figure out where the work will be
cc @matentzn and @cthoyt