-
Notifications
You must be signed in to change notification settings - Fork 8
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
COB Revised -- Wait for James to Merge #288
base: master
Are you sure you want to change the base?
Conversation
This revision is based on the results of the September 2024 COB workshop. Changes have not yet been vetted.
Looking at the diff, I already see some changes I want to make:
|
@cmungall I'd appreciate your feedback on the overall approach here. |
I'm afraid I only have time now for a cursory look. Minor: cob-to-external is now subsumed into cob-edit. This makes sense (we should make sure to sync docs in this PR or closely after). It looks like not everything is accounted for. E.g. we had a lot of narrow classes e.g ZFA roots. Maybe these are of less importance but I think we should have a trail here, maybe leave behind a rump cob-to-external with the entries not in cob-edit Domains and ranges: we still need to solve this problem (#213). This PR improves over the current release (which actually injects domains and ranges that are too strict). |
I made a few improvements:
@sebastianduesing Can you please sanity check the |
I went through those three files to check for the following:
Everything looks good on points 1 and 2. While checking on point 3, I noticed a problem: I think this mix-up started in the workshop Google Sheet; I've now fixed it there. Let me know if you'd like me to add a commit here to fix it in |
Thanks @sebastianduesing! I think this is in good enough shape to share and discuss. Still to do:
|
GitHub was complaining about "illegal quoting", so this might make it happier.
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.
This PR constitutes a critical improvement for COB moving forward. I endorse all the proposed changes from a high level - but I admittedly leave details to my trust in @jamesaoverton diligence in all things (sorry).
With this review I am assuming someone scrolled through the cob.owl diff and convinced themselves all changes are expected - I didn't do that (just eyeball, lgtm).
I reviewed cob-root.owl and cob-base.owl They look great, clean and all! I personally find it a little unfortunate that not all COB classes have a definition, e.g. cellular organism
or the all dangerous characteristic
, but that is not really key to this PR here.
I made a few minor comments, but I don't think they are critical to be addressed before merging this.
I noticed today that it appears that IAO:0000109 "measurement datum" was dropped from this version of COB. I went back to the workshop sheet to check if that's just one we hadn't made a decision about, but I didn't find it in that sheet either. I may have forgotten a conversation we've had about that one—was that an intentional omission? I've got no stake on whether it's in there or not, just want to make sure I didn't miss something. |
By robot-exporting this version and the existing version of COB and diffing the TSVs, I found a couple more terms that appear to be unaccounted for in this version (ignoring terms that were simply not decided on at the workshop):
Let me know if you'd like me to add a commit to fix any/all of these. |
I would prefer to keep the label as 'is specified output of' - we should be
conservative in making label changes unless there is a good argument. And
while it is a bit more verbose, I think it helps with immediately
recognizing what domain and range should be.
…On Wed, Dec 11, 2024 at 9:50 AM Sebastian Duesing ***@***.***> wrote:
By robot-exporting this version and the existing version of COB and
diffing the TSVs, I found a couple more terms that appear to be unaccounted
for in this version (ignoring terms that were simply not decided on at the
workshop):
- 'mass' and 'charge' were both placed in full according to the
workshop sheet, but do not appear in this branch.
- 'is specified input of' is missing in this branch.
- Adjacently, COB currently changes the label of 'is specified output
of' to 'specified output of'. I can't recall whether this was an
agreed-upon change.
Let me know if you'd like me to add a commit to fix any/all of these.
—
Reply to this email directly, view it on GitHub
<#288 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADJX2IU6J67HARNE3NKCMLD2FB3MRAVCNFSM6AAAAABQRZVOIGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMZWGY4TIMZUGM>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
--
Bjoern Peters
Professor
La Jolla Institute for Immunology
9420 Athena Circle
La Jolla, CA 92037, USA
Tel: 858/752-6914
Fax: 858/752-6987
http://www.liai.org/pages/faculty-peters
|
I made a few updates to address comments by @sebastianduesing:
I also restored the generated |
This revision is based on the results of the September 2024 COB workshop.
Update: The upshot of these changes is to decide which classes should be in the COB namespace and which should remain in their source namespace. Previous versions of COB were not careful or consistent about this.
Changes have not yet been vetted.Update: This PR has been public for 6 weeks. It has been reviewed by two senior OBO Technical Working Group members. OBO Operations members have been specifically asked to review if interested. We are now asking the OBO Discuss mailing list for comment.
We need to compare to the previously releasedcob.owl
file and ensure that we aren't dropping any important terms.Update: We have carefully compared to the previously released
cob.owl
and are satisfied that changes are intentional.The biggest change here is a
src/ontology/cob-edit.tsv
file, which is a ROBOT template that replacescob-edit.owl
. Given COB's small size and tight integration, I think that a template is a good choice for editing, butthis needs discussionwe could reverse course. I may have bent ODK conventions to the breaking point.The
src/scripts/split-cob-edit.py
script splits thecob-edit.tsv
into three "modules":Update: COB workshop term review sheet
Update: Direct links to download the OWL files:
I think that the most controversial part of the current version is the 'REPLACED' and 'REPLACEMENT' object properties. I added temporary object properties such as 'has material part' with tight domains and ranges, and used these in the logical axioms of the terms in the sheet. The
split-cob-edit.py
script replaces these temporary object properties with existing object properties, such as RO 'has part'. The existing object properties may have domains and ranges that we were trying to leave out of COB, such as 'continuant'.