oborel / obo-relations

RO is an ontology of relations for use with biological ontologies
http://oborel.github.io/
Other
91 stars 46 forks source link

NTR 'contains measured amount' #726

Closed jmwhorton closed 8 months ago

jmwhorton commented 1 year ago

Addresses new term request: #712 (original label was 'has actual load')

Please note: imports were added manually to other_import.owl. I couldn't see another place that made more sense. If I have added the imports incorrectly, please let me know and I'm happy to make the change.

wdduncan commented 1 year ago

@jmwhorton please adress the following:

  1. We can discuss at the next meeting, but I would prefer the label to more explicitly reflect that the range is a measurement data. E.g., "container amount measured by". "contains amount" can still be a synonym (e.g., has_exact_synonym.

  2. Change

    A relation that relates a container to a measurement datum that specifies the actual amount of material in the container.

    to

    A relation between a container and measurement datum that specifies the actual amount of material in the container.
  3. Instead of term editor (IAO_0000117), use annotation dcterms:contributor and the editor's ORCID. Each contributor gets their own contribution annotation.
    As an example see positively correlated with. (You will have to open in Protege. OLS doesn't pull up the term.)

  4. Can you please provide an example of usage.

  5. The definition source annotation doesn't seem relevant here. "Contains" is a common word and the contributors are listed if anyone has questions.

  6. Is there an inverse relation? E.g., "measures container amount"

nataled commented 1 year ago

@wdduncan I'm confused by your suggested term label change. Isn't the relation meant to be used like "this beaker 'contains amount' 400 ml"? I think the suggested change fits better with how something is being measured, like "this beaker 'container amount measured by' Mettler scale".

wdduncan commented 1 year ago

@nataled I see your point.

"this beaker 'container amount measured by' Mettler scale".

I made the suggestion b/c I was trying to make it more clear that the range is a measurement datum and not a literal.

nataled commented 1 year ago

@wdduncan substituting 'as' in place of 'by' might come a bit closer (in my mind, at least) in evoking the correct range type. Even so, the 'as' version could just as likely have someone think that 'weight' or 'volume' would be an appropriate response. I'm not sure how best to do it.

jmwhorton commented 1 year ago

I have addressed numbers 2, 3, and 5 from @wdduncan's list above. For the rest, I'll defer to the group consensus and ask @cstoeckert for an opinion.

wdduncan commented 1 year ago

@nataled I've thinking about this past few days, and I don't have any real answer regarding container amount measured by vs. container amount measured as. We can discuss at tomorrow's RO meeting. But here are some random thoughts.

  1. It would be nice if the label contains amount had a nice inverse. Not sure what this is though ... Perhaps amount in container?
  2. Normally, I'd be okay with using the label contains amount. But, there has been recent discussion about including data properties in RO. Maybe I'm worrying to much, and we should just use the label as is. If a data property for amounts in containers is proposed, we can address it then. (i.e., for now we kick the can down the road).

In any case, let's focus on wrapping this up at the meeting. This issue doesn't need to hang around too long.

wdduncan commented 1 year ago

Agreed on RO call to change label to contains measured amount.

wdduncan commented 11 months ago

Fixes #712

jmwhorton commented 11 months ago

Added example of usage. I believe all issues have been addressed, and this is ready to be reviewed again.

wdduncan commented 11 months ago

Thanks @jmwhorton
Did you manually add the domain (device) and range (measurement data item] to obi_import.owl manually?

If so, I believe those terms should be added to the obi_terms.txt file, and the import should be rebuilt. Otherwise, the changes you made to obi_import.owl will get overwritten by running ODK.

@matentzn @anitacaron can you confirm this? I can't quite recall the ODK command to rebuild and OBI import. I think it is:sh run.sh make refresh-obi. However, is it better to update the whole repo, i.e., sh run.sh make update_repo?

wdduncan commented 11 months ago

P.S. I just noticed @anitacaron comment. Not sure if it better to run sh run.sh make refresh-obi or sh run.sh make imports/obi_import.owl. I think they may do the same thing.

jmwhorton commented 11 months ago

For the sake of clarity I'll answer both the previous commends, @wdduncan. I didn't add the imports to obi_import.owl manually. I did run make imports/obi_import.owl

However, do note that I added terms into other_import.owl manually (when I originally made the change, all of the imports were added there. I manually added them to the .owl file since other_terms.txt was empty).

Happy to make any changes if I need to.

wdduncan commented 11 months ago

Thanks @jmwhorton !
I believe the new terms should go in obi_terms.txt. Let's wait on @anitacaron or @matentzn to confirm.

jmwhorton commented 11 months ago

To be specific, the new terms that I left in other_terms.owl are IAO terms, not OBI terms. (so I didn't want to put them in the OBI file)

wdduncan commented 11 months ago

Thanks. I didn't catch that. Ideally, we should create new IAO import, but measurement data item is in OBI ... What you think @anitacaron ?

anitacaron commented 11 months ago

You're right, @wdduncan. We need to create an IAO import and add the IAO term there instead of adding it to the other_import.owl file.

wdduncan commented 11 months ago

@anitacaron Can you modify the ODK file to create the IAO import? Also, is there documentation about the purpose of other_import.owl?

ddooley commented 11 months ago

One question - can the IAO terms be imported by way of COB? I recall there was going to be some change made to COB import to enable 3rd party terms imported that way?

wdduncan commented 11 months ago

@anitacaron will be able to modify the ro-odk.yaml so that IAO is included as an import?
Also, do you have any documentation on what other_import.owl is used for?

anitacaron commented 11 months ago

@wdduncan I think the other_imports.owl was created to have all imports, but it was manually updated. There's a comment in the file.

Annotation(rdfs:comment "Ad-hoc additional imports, manually copied. This should be split into several dynamically imported sub-modules.")

Ideally, we should remove this file. Now it only imports a few terms. It would be good to review, update, and create a proper import in another issue. For example, instead of disease (OGMS), use disease (MONDO).

Can the IAO terms be imported by way of COB?

@jmwhorton, now that we have the COB import, you can import the COB terms like OBI. Add the terms in the src/ontology/imports/cob_terms.txt file and run sh run.sh make imports/cob_import.owl I found measurement datum - COB:0000121 and data item - IAO:0000027. But only the measurement datum is used in the new RO property, so I think you only need to add this term to the cob_terms.txt file.

wdduncan commented 11 months ago

@jmwhorton Have had a chance to look at the changes? What is still needed to move this forward?

jmwhorton commented 10 months ago

Requested changes have been made, @wdduncan

jmwhorton commented 10 months ago

@anitacaron 'dc' has been replaced with 'terms' now to fit with recent changes to master.

wdduncan commented 8 months ago

@anitacaron I don't understand which changes you requested. Have they been addressed?

anitacaron commented 8 months ago

Yes, they were addressed, and all conversations were resolved. I'm waiting for someone from the ontology to approve it, and I'll approve it after.

wdduncan commented 8 months ago

Yes, they were addressed, and all conversations were resolved. I'm waiting for someone from the ontology to approve it, and I'll approve it after.

Ok. I approved.

wdduncan commented 8 months ago

Thanks @anitacaron !!!!