search

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

measures characteristic #717

Closed ddooley closed 3 months ago

ddooley commented 1 year ago

and inverse 'characteristic measured by'

wdduncan commented 1 year ago

Since the the relation holds between a process and a quality, do we to make the label more reflective of that? E.g.:

This would make it clear that it is not a datum that holds measurement information.

wdduncan commented 1 year ago

Since the COB import has been added to the ODK file, I think this addresses #716.
Do others agree?

ddooley commented 1 year ago

Re "process measures characteristic" label etcl, I feel it is intuitive that "measures" is tied to a measuring process, so no need to spell that out in label.

wdduncan commented 1 year ago

I agree 'measures' brings to mind a process, but I thought it may need to be made clearer what is doing the measuring. E.g., When I step on scale, there is a measurement that happens. But, just reading the label measures characteristic, one might not understand whether it is it the scale (i.e., device) that measures my weight or the applying of pressure (i.e., process) that does the measuring?
For the measures characteristic relation, it is the latter, but I can see how someone might think it is the former (i.e., device).

bpeters42 commented 1 year ago

I added my comments to the issue rather than this pull request so that they can viewed in context.

ddooley commented 1 year ago

I'm going to have to redo this one. Its getting too complicated trying to resolve diffs with pre-UBERON integration

anitacaron commented 1 year ago

Hi @ddooley, I'll update this PR to align with the master. You don't need to worry about redoing the work.

anitacaron commented 1 year ago

Note: COB import is empty. If there isn't a COB term to import, I'd recommend removing the COB import.

ddooley commented 1 year ago

Yes I guess that COB import can be removed. I'd put it in thinking I could import OBI terms that way, but to my surprise one can't fetch 3rd party terms from COB indirectly (as mentioned in last RO meeting).

matentzn commented 1 year ago

You will end up adding COB again later.. I would leave it. And also, someone needs to sort out the issue of "who maintains mapped COB terms" urgently..

anitacaron commented 1 year ago

There's another issue to address the COB import #716. It will be added when needed.

wdduncan commented 1 year ago

Can you leave the COB import? I need it for one of my PRs. This will keep me from having to recreate it.

anitacaron commented 1 year ago

I'll add it back, fixing issue #716.

wdduncan commented 1 year ago

Thanks @anitacaron

wdduncan commented 11 months ago

The label reads process measures characteristic, but the domain is assay. Should the label be assay measures characteristic?

The range is PATO:quality. This would exclude BFO:quality.

image

Did you intend the range to be specifically dependent continuant?

ddooley commented 11 months ago

The range was meant to be COB characteristic, but for now that translates to PATO:00000001 quality.

As for domain, @bpeters42 I recall you suggested "assay" for domain since that was doing the measuring, and I recall assay can't measure nonmaterial things like rates directly, but our solution is that assay can measure material things that are proxies for things like rates of change. So I've changed label to "assay" on that basis. Unfortunately it added some superfluous diffs too tho.

wdduncan commented 11 months ago

In COB, the IRI for characteristic points to specifically dependent continuant. So, using SDC as the range should be fine.

However, we should verify if the IRI for characteristic has been settled on. E.g., Will it be changed to PATO:quality in the future.

ddooley commented 11 months ago

@anitacaron want me to just do a redo on this one? Might be simpler.

anitacaron commented 11 months ago

I'll import COB characteristic (BFO:0000020).

anitacaron commented 11 months ago

@ddooley you can change the range to BFO:0000020 now.

ddooley commented 11 months ago

Switch to BFO:0000020 is now done!

ddooley commented 11 months ago

I think @bpeters42 conversation about big diff is also taken care of with your work.

github-actions[bot] commented 8 months ago

This PR has not seen any activity in 90 days and has been marked as stale. If it is no longer needed, please close the PR. Otherwise, please update the PR with a status update.

ddooley commented 5 months ago

I did another merge of master into this branch and I think outstanding issues are taken care of?

ddooley commented 4 months ago

@wdduncan @bpeters42 final touchups are finished!

bpeters42 commented 3 months ago

I think the pull request as is doesn't do much harm; there is still a lot to be resolved for when we transition to modeling values of measurements via @jamesaoverton approach, but this will not make that worse, and it has been requested for a while. So I would approve. (apart from the minor typo I found)

jamesaoverton commented 3 months ago

@bpeters42 I have concerns about the bigger picture here, but I agree that this PR should go ahead.