Skip to content
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

Documentation: no NTR should ever subclass a non-ENVO class #1226

Open
cmungall opened this issue Oct 27, 2021 · 13 comments
Open

Documentation: no NTR should ever subclass a non-ENVO class #1226

cmungall opened this issue Oct 27, 2021 · 13 comments

Comments

@cmungall
Copy link
Member

If you are making an NTR to ENVO, every one of your terms MUST subclass at least one ENVO class (or use an ENVO class in a genus) and MUST NOT assert a subclass axiom to any non-ENVO class

Only core ENVO editors should ever subclass to BFO, CHEBI, OBI, etc. If you think you really need to subclass a non-ENVO class, open a specific issue just for this one case and we will discuss. In some cases it may transpire we need to better document our DPs.

For example, we should not subclass CHEBI. Instead subclass material, and used composed-primarily-of to link to CHEBI. ENVO represents a different level of granularity than CHEBI.

(Furthermore, I think even core ENVO editors should ONLY subclass ENVO or COB, but that is for another issue)

Rationale: subclassing non-ENVO classes is poor modularization and leads to issues. Additionally the axioms are likely wrong (see my comments on bottle in #1105)

Aside: OWL is a very powerful language that allows you to do all kinds of interesting things weaving together different ontologies, but a good rule of thumb is keep it simple and avoid trying to do anything clever.

Indicate whether you are in favor with a comment +1, -1, +0 with any additional comments.

@kaiiam
Copy link
Contributor

kaiiam commented Oct 28, 2021

👍 to reducing reciprocal dependencies where possible and minimizing the use of subclassing non ENVO. However, I understand there might still be cases where it's useful. So I reiterate the general idea that terms should strive to follow some sort of agreed upon design pattern where possible.

For a case like @ddooley's natural-based polymers which he's wanting to add as subclass to CHEBI macromolecule. @cmungall suggestion would instead be to add the term as subclass to environmental material then link to composed-primarily-of CHEBI macromolecule? I presume we could have a generic design pattern for that. I also don't think we should change our standard for labels to have things like natural-based polymers (FOODON:03500057).

@cmungall is also proposing in #945 that we remove the foodon import in that case we probably don't want to be linking off to foodon in ENVO. If foodon needs this linkage perhaps they can import the ENVO term and link to it there?

# Class: <http://purl.obolibrary.org/obo/ENVO_03501303> (natural-based polymers (FOODON:03500057))

AnnotationAssertion(Annotation(<http://www.geneontology.org/formats/oboInOwl#hasDbXref> <https://www.sciencedirect.com/topics/materials-science/natural-polymer>) <http://purl.obolibrary.org/obo/IAO_0000115> <http://purl.obolibrary.org/obo/ENVO_03501303> "Macromolecules that are obtained naturally from plants or animals."@en)
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#created_by> <http://purl.obolibrary.org/obo/ENVO_03501303> <https://orcid.org/0000-0001-5275-8871>)
AnnotationAssertion(<http://www.geneontology.org/formats/oboInOwl#creation_date> <http://purl.obolibrary.org/obo/ENVO_03501303> "2021-03-15T04:00:01.750Z"^^xsd:dateTime)
AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/ENVO_03501303> "natural-based polymers (FOODON:03500057)"@en)
SubClassOf(<http://purl.obolibrary.org/obo/ENVO_03501303> <http://purl.obolibrary.org/obo/CHEBI_33839>)

@ddooley
Copy link
Contributor

ddooley commented Oct 28, 2021

Ok, I now get the concept that we should not subclass terms under ENVO if the term parent belongs to a branch that was actually imported from another ontology, e.g. CHEBI "chemical entity" branch in ENVO; I was fuzzy on that over a year ago. As well the term label (FOODON:...) brackets were accidental, and the plural too on "natural-based polymers".

Shall I make the label and parent changes on current pull request (which should trigger a new approval process?) OR I'll make the changes asap on a new request after pull goes in?

My big learning on this one is not to batch so many term requests in one go. From now on much smaller bundles.

@ddooley
Copy link
Contributor

ddooley commented Oct 28, 2021

And yes @kaiiam to: " If foodon needs this linkage perhaps they can import the ENVO term and link to it there?" We can implement this shortly.

@kaiiam
Copy link
Contributor

kaiiam commented Oct 29, 2021

Shall I make the label and parent changes on current pull request (which should trigger a new approval process?) OR I'll make the changes asap on a new request after pull goes in?

I feel like it'd be better not to do the latter, but that's just me. I'd suggest either fixing the existing PR or regenerating a new one if that's easier. Whatever's easiest for you. Were you perchance using the https://github.com/EnvironmentOntology/envo/wiki/ENVO-Robot-template-and-merge-workflow for a robot template of terms? Might help.

@cmungall
Copy link
Member Author

cmungall commented Nov 1, 2021

Shall we discuss the strategy for your PR over of #1105?

@kaiiam
Copy link
Contributor

kaiiam commented Nov 1, 2021

Yes sorry we were conflating issues. This thread is about not subclassing NON envo:

Recap:

@cmungall: "Never subclass NTRs to non ENVO classes unless it's BFO/COB".

@kaiiam: "Sounds reasonable as our standard practice, but maybe we allow occasional exceptions if there's a good reason for it?"

@pbuttigieg @wdduncan @turbomam @ddooley thoughts?

@wdduncan
Copy link
Member

wdduncan commented Nov 1, 2021

@kaiiam Yes, I agree with this. I am suspicious about allowing for adding class directly under BFO/COB. EnvO seems pretty filled out, and discussion may be warranted if someone is adding a new class at that level of generalization.

@wdduncan
Copy link
Member

wdduncan commented Nov 1, 2021

+1 (see comment above)

@ddooley
Copy link
Contributor

ddooley commented Nov 2, 2021

And now that I understand the subclassing under 3rd party issue, I recall this is coming up especially in application ontologies within the foundry. I'll do my best to correct it within my areas, but to note it is principle that I've learned too recently, so maybe it is in OBO Foundry documentation somewhere, but if it could be brought to the fore, that would be good. There are related MIREOT topics - [like where to put people in roles](e.g. kids of https://www.ebi.ac.uk/ols/ontologies/obi/terms?iri=http%3A%2F%2Fpurl.obolibrary.org%2Fobo%2FNCBITaxon_9606) - that require decisions! The principle itself puts pressure on terms to migrate to their most appropriate ontology parent class.

@pbuttigieg
Copy link
Member

pbuttigieg commented Nov 12, 2021

+0

If the new class is in scope of ENVO but doesn't fit in any of its lower-level hierarchies, then why not?

Only core ENVO editors should ever subclass to BFO, CHEBI, OBI, etc. If you think you really need to subclass a non-ENVO class, open a specific issue just for this one case and we will discuss. In some cases it may transpire we need to better document our DPs.

That's reasonable, given that it's a domain management thing and thus policy-level.

(Furthermore, I think even core ENVO editors should ONLY subclass ENVO or COB, but that is for another issue)
Rationale: subclassing non-ENVO classes is poor modularization and leads to issues. Additionally the axioms are likely wrong (see my comments on bottle in #1105)

OBO is imperfectly modularised, so until that gets better, we can expect this to happen from time to time, even though we should avoid it.

@cmungall
Copy link
Member Author

OK, it seems we are agreed that any proposed non-ENVO subclass should first be made into its own ticket and a case made why an exception should be made, and should not be folded into a larger PR

@wdduncan
Copy link
Member

Since we are agreed, do you want to leave the issue open or wait and close until it makes it into the documentation?

@kaiiam
Copy link
Contributor

kaiiam commented Apr 4, 2022

Lets leave this open until it goes into the docs. I've added the documentation label so we can find it later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants