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

Refresh imports for Uberon release #3461

Merged
merged 2 commits into from
Jan 14, 2025
Merged

Refresh imports for Uberon release #3461

merged 2 commits into from
Jan 14, 2025

Conversation

aleixpuigb
Copy link
Collaborator

No description provided.

@aleixpuigb aleixpuigb self-assigned this Jan 13, 2025
Copy link

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

http://purl.obolibrary.org/obo/UBERON_0011940 in taxon http://purl.obolibrary.org/obo/NCBITaxon_9606 SubClassOf Nothing

Axiom Impact

Axioms used 1 times

Ontologies used:

@balhoff
Copy link
Member

balhoff commented Jan 13, 2025

The error above is almost exactly the example I used in the taxon constraints explainer! 😁 https://oboacademy.github.io/obook/explanation/taxon-constraints-explainer/#using-taxon-restrictions-for-quality-control

@aleixpuigb
Copy link
Collaborator Author

@emquardokus this conflict is caused by refreshing the HRA subset. The addition of arrector pili muscle of vibrissa in one of the ASCT+B tables results in the addition of 'present in taxon' some 'Homo sapiens':

image

This term is part_of some strand of vibrissa hair that contains `'never in taxon' some Homo sapiens'.

As we need an Uberon release and fixing this will take some time (it needs to be fixed externally to Uberon), I think the best course of action for now is close this PR and refresh the imports in a new one without refreshing the HRA subset.

cc @gouttegd @dosumis

@dosumis
Copy link
Contributor

dosumis commented Jan 14, 2025

Pretty clearly an error in the ASCT+B tables (unless someone has discovered that humans have whiskers)

@gouttegd
Copy link
Collaborator

@aleixpuigb Since it seems that it is clearly an error in the HRA subset, what you can also do is simply edit the refreshed HRA subset file to remove the offending axiom.

We normally don't recommend this kind of fix (it's only a band-aid as the real fix must happen upstream), but this is an instance where it is actually justified.

This term was wrongly added into HRA (human don't have whiskers) and it needs to be removed from the ASCT+B tables
@aleixpuigb aleixpuigb requested a review from gouttegd January 14, 2025 10:37
Copy link
Collaborator

@gouttegd gouttegd left a comment

Choose a reason for hiding this comment

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

Looks all good to me.

@aleixpuigb aleixpuigb merged commit 41c24df into master Jan 14, 2025
1 check passed
@aleixpuigb aleixpuigb deleted the refresh_imports branch January 14, 2025 11:32
@@ -153265,30 +154006,20 @@ AnnotationAssertion(rdfs:comment <http://purl.obolibrary.org/obo/PR_000050216> "
AnnotationAssertion(rdfs:label <http://purl.obolibrary.org/obo/PR_000050216> "receptor-type tyrosine-protein phosphatase C isoform CD45R")
SubClassOf(<http://purl.obolibrary.org/obo/PR_000050216> <http://purl.obolibrary.org/obo/PR_000001006>)

# Class: <http://purl.obolibrary.org/obo/PR_000050567> (obsolete protein-containing material entity)
# Class: <http://purl.obolibrary.org/obo/PR_000050567> (protein-containing material entity)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am conducting the release (I am reviewing the diff file) and realised that many terms were adding SubclassOf PR_000050567. I am not sure why it has reverted this obsoletion.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Last time this happened it came from the PR slim import. Might be worth checking if anything changed over there recently...

(In a train right now, can't do anything.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes it comes from there, I will try to investigate
image
Thank you!

Copy link
Collaborator

Choose a reason for hiding this comment

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

The non-obsoleted PR:000050567 was apparently re-introduced in the PR slim yesterday as part of this commit.

Not entirely sure how it could have happened, but my wild guess is that there might have been a glitch when the upstream pr.owl.gz file was being downloaded, which may have caused the mirror not to be updated, so the slim may have been re-generated using an old mirror that dated from before PR:000050567 has been fixed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If my wild guess is true, it’s an issue that needs to be addressed in the PR slim repository. The process of re-generating the slim must be made to be more robust to this kind of glitches.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For now, I’d suggest

(1) re-generating the PR slim after making sure you forcefully delete the local PR mirror (everything in the mirror directory);
(2) checking that PR:000050567 is obsoleted as it should in the re-generated slim;
(3) push the changes to the PR slim repo;
(4) refresh the imports in Uberon once again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you for looking into this Damien.

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

Successfully merging this pull request may close these issues.

4 participants