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

multicellular organism disjoint from organism substance #2421 #3151

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ddooley
Copy link

@ddooley ddooley commented Dec 14, 2023

Addresses Issue #2421 . This is in external disjoints file though the two terms are native to Uberon though.

@ddooley ddooley requested a review from anitacaron as a code owner December 14, 2023 04:23
@anitacaron anitacaron linked an issue Dec 15, 2023 that may be closed by this pull request
@anitacaron
Copy link
Collaborator

GitHub action is having issues posting the comment because it comes from a fork, but there're unsats in this PR.

This PR violates some taxon constraints. Here is what the reasoner has to say:

Barley yellow dwarf virus SubClassOf Nothing

Luteovirus pavhordei SubClassOf Nothing

Barley yellow dwarf virus (ISOLATE MAV-PS1) SubClassOf Nothing

Histoplasma capsulatum var. duboisii H88 SubClassOf Nothing

median vaginal canal SubClassOf Nothing

lateral vaginal canal SubClassOf Nothing

Barley yellow dwarf virus (ISOLATE P-PAV) SubClassOf Nothing

Barley yellow dwarf virus MAV SubClassOf Nothing

Luteovirus mavhordei SubClassOf Nothing

Bean leafroll virus SubClassOf Nothing

Luteovirus phaseoli SubClassOf Nothing

Barley yellow dwarf virus (ISOLATE NY-RPV) SubClassOf Nothing

Barley yellow dwarf virus PAV SubClassOf Nothing

Luteovirus SubClassOf Nothing

unclassified Luteovirus SubClassOf Nothing

Axiom Impact

Axioms used 12 times

Axioms used 3 times

Axioms used 2 times

Axioms used 1 times

Ontologies used:

Copy link
Collaborator

@anitacaron anitacaron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix unsats.

@gouttegd
Copy link
Collaborator

The obvious problem in Uberon is here:

With “channel for” being range-restricted to “organism substances”, it is obviously a mistake to say the vaginal canal is a “channel for” the embryo. Either the range restriction on that relation should be relaxed if the intention is for it to be used for more than just substances, or the relationship between “median vaginal canal” and “embryo” should either be dropped or replaced by a more suitable relation.

@gouttegd
Copy link
Collaborator

But I am puzzled by the other unsats, which don’t seem to have anything to do with Uberon at all.

@gouttegd
Copy link
Collaborator

I can’t reproduce the other unsats locally. The only unsats I get running the test suite locally are the ones caused by median vaginal canal as explained above.

Copy link

github-actions bot commented Feb 2, 2024

This PR has not seen any activity in the past month; if nobody comments or reviews it in the next week, the PR creator will be allowed to proceed with merging without explicit approval, should they wish to do so.

@cmungall
Copy link
Member

cmungall commented Feb 2, 2024

The NCBITaxon unsats likely due to superimposition of 2 versions of NCBITaxon (this is one of the key motivations for base files). Difference between local and actions may be due to difference in versions on the network and/or triggering rebuilds of mirrors (but I defer to the Damien with an E who knows ODK better than me)

As for channeling embryos, rococo/vanity axiom, remove it

@gouttegd
Copy link
Collaborator

gouttegd commented Feb 2, 2024

@ddooley Can you remove the “median vaginal canal SubClassOf channel for some embryo“ axiom in Uberon? It makes sense that this should be done as part of this PR, since the incorrectness of this axiom was revealed by the addition of your disjointness axiom between “multicellular organism“ and “organic substance“.

I would also suggest that said disjointness axiom should be added to the main Uberon source file (uberon-edit.obo) rather than the external-disjoints.obo component. That component is only merged at build time, so its axioms are not visible to editors at edit time (e.g. in Protégé). This creates a risk than an editor could later violate the disjointness with no way of realising it until they submit a PR and the violation is caught by the test suite.

@obophenotype obophenotype deleted a comment from github-actions bot Mar 8, 2024
@obophenotype obophenotype deleted a comment from github-actions bot Apr 8, 2024
Copy link

github-actions bot commented May 9, 2024

This PR has not seen any activity in the past month; if nobody comments or reviews it in the next week, the PR creator will be allowed to proceed with merging without explicit approval, should they wish to do so.

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

Successfully merging this pull request may close these issues.

multicellular organism and organism substance should be disjoint
4 participants