-
Notifications
You must be signed in to change notification settings - Fork 91
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
first pass at howto for ontologies as values #1585
base: main
Are you sure you want to change the base?
Conversation
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.
Fixed some typos
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
GenePhenotypeAssociation: | ||
attributes: | ||
gene: | ||
range: Gene |
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.
Where did the pattern
go? Shouldnt it be reflected somewhere?
GenePhenotypeAssociation: | ||
attributes: | ||
gene: | ||
range: Gene |
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.
See below, its unclear why this pattern match constraint suddenly disappeared.
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.
Conversely, as a naive reader, I would expect to be able to add the constraint on the Gene
class (I guess through a mapping of some kind?). Maybe keep the example isomorphic here, and when you extract a class, make sure the model does still validate the same kinds of associations as before.
docs/howtos/ontologies-as-values.md
Outdated
is_a: NamedThing | ||
``` | ||
|
||
Let's add a container class: |
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.
Maybe add a sentece why we would want to do that at this point, since we are still expecting to model a simple g2p association
``` | ||
Container: | ||
attributes: | ||
genes: |
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 can see why we need an associations
container now, but not so much a genes
container, at least not for the running example (EDIT I see it becomes clearer a bit later)
docs/howtos/ontologies-as-values.md
Outdated
``` | ||
|
||
We can go one step further, and represent the ontology | ||
hierarchy in the data, by adding a `parents` slot in the schema: |
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.
Why would we want that if the hierarchy is already in the ontology (add a remark explaining why one would want to do that, given that the hierarchy already exists in the ontology)
|
||
This is very practical - consumers of the data can consume the | ||
associations and the ontology hierarchy together to perform rollup | ||
operations, etc. |
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.
Needs 1 sentence more to be convincing
|
||
The fact that we have two classification systems co-existing (LinkML | ||
is_a hierarchy and ontology hierarchy as data) should not be a cause | ||
for concern. |
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.
need a because..
clause
docs/howtos/ontologies-as-values.md
Outdated
In fact this is quite straightforward - ontology classes (typically | ||
formalized in OWL) and classes in LinkML are not the same thing, | ||
despite the name "class". And instances in LinkML and instances in | ||
realist OBO ontologies are not the same thing. |
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.
Why realist
? are instances in OBO ontologies that are not realist (in the sense of, BFO aligned), the same as instances in LinkML? Maybe drop that word - most people would not even know what it means.
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.
It's tricky. This part of the guide is aimed primarily at people who may be coming from an OBO/Realist viewpoint. Those that haven't been exposed to this way of thinking generally have no issues thinking of ontologies as data.
docs/howtos/ontologies-as-values.md
Outdated
despite the name "class". And instances in LinkML and instances in | ||
realist OBO ontologies are not the same thing. | ||
|
||
So long as we realize this, there isn't any issue. Again, this is |
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.
You say: the two are different. If this is the realisation you are looking for, then I agree, its clear! But I don't understand the exact point of this whole paragraph here.. (Row 272-275)
Addressing @matentzn's comments
Co-authored-by: Nomi Harris <nlharris@users.noreply.github.com>
Can this be merged or is it waiting for Chris to address Nico's comments? |
No description provided.