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

add qc workflow #1240

Merged
merged 4 commits into from Jan 21, 2022
Merged

add qc workflow #1240

merged 4 commits into from Jan 21, 2022

Conversation

wdduncan
Copy link
Member

No description provided.

@wdduncan
Copy link
Member Author

@cmungall I ran make test locally (which is called in the qc.yml workflow. It found a number of equivalence errors (see below). Do you want to merge this now, or fix these errors first?

2021-11-15 20:46:45,984 ERROR org.obolibrary.robot.ReasonOperation - No equivalent class axioms are allowed
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_00001142> == <http://purl.obolibrary.org/obo/FOODON_00001765>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_03301103> == <http://purl.obolibrary.org/obo/FOODON_00001100>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_00001142> == <http://purl.obolibrary.org/obo/FOODON_03310791>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_03460231> == <http://purl.obolibrary.org/obo/FOODON_03460229>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/ENVO_01001479> == <http://purl.obolibrary.org/obo/ENVO_01001784>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_00001765> == <http://purl.obolibrary.org/obo/FOODON_03310226>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_00001765> == <http://purl.obolibrary.org/obo/FOODON_03310791>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_00001142> == <http://purl.obolibrary.org/obo/FOODON_03310226>
2021-11-15 20:46:45,985 ERROR org.obolibrary.robot.ReasonOperation - Equivalence: <http://purl.obolibrary.org/obo/FOODON_03310226> == <http://purl.obolibrary.org/obo/FOODON_03310791>
make: *** [envo-full.owl] Error 1
rm imports/fao_combined_seed.tsv imports/chebi_combined_seed.tsv imports/pato_combined_seed.tsv imports/go_combined_seed.tsv imports/pco_combined_seed.tsv imports/po_combined_seed.tsv imports/ro_combined_seed.tsv imports/ncbitaxon_combined_seed.tsv imports/uberon_combined_seed.tsv imports/iao_combined_seed.tsv imports/obi_combined_seed.tsv

@cmungall
Copy link
Member

Sigh, I guess these crept in while we wre in the phase where we had no checking of PRs and things were merged in?

You you show explanations for each (robot explain)

The foodon ones are unusual - envo should not cause foodon to be unsatisfiable, could it be

  • superimposition of different releases
  • foodon latest release is incoherent

@wdduncan
Copy link
Member Author

@cmungall I don't know the provenance of how the offenders made it into EnvO. I can create a ticket to focus on fixing these issues.

It is also worth noting that make test calls owltools. So that will have to be installed into the PR image.

@wdduncan wdduncan marked this pull request as draft November 16, 2021 17:01
@cmungall
Copy link
Member

owltools is in ODK

@cmungall
Copy link
Member

cmungall commented Nov 16, 2021

image

@ddooley ^^^

@cmungall
Copy link
Member

do we have a ticket about removing the foodon import

currently it's only used in 3 places:

✗ grep FOODON envo-edit.owl | grep -v ^Ann
EquivalentClasses(<http://purl.obolibrary.org/obo/ENVO_01000929> ObjectIntersectionOf(<http://purl.obolibrary.org/obo/ENVO_2100000> ObjectSomeValuesFrom(<http://purl.obolibrary.org/obo/RO_0002507> <http://purl.obolibrary.org/obo/FOODON_00001287>)))
SubClassOf(<http://purl.obolibrary.org/obo/ENVO_02000047> <http://purl.obolibrary.org/obo/FOODON_00002403>)
SubClassOf(<http://purl.obolibrary.org/obo/ENVO_02000055> <http://purl.obolibrary.org/obo/FOODON_00002403>)

@cmungall
Copy link
Member

@wdduncan I suggest for now we switch off equivalence checking, get this merged, and then make a PR to re-introduce it, which we do as soon as foodon is fixed

@ddooley
Copy link
Contributor

ddooley commented Nov 17, 2021

Hmm. the hominy equivalence is wrong - probably intended to be a conjunction, not a disjunction but even so, still not sufficient differentia. I've dropped it and will do new release shortly.

@ddooley
Copy link
Contributor

ddooley commented Nov 17, 2021

I have dropped the hominy equivalence; It may be that all the above "no equivalent axioms are allowed" items are eliminated as a result.

@cmungall
Copy link
Member

This has become urgent - where do we stand with this?

@ddooley
Copy link
Contributor

ddooley commented Jan 18, 2022

From FoodOn's side a refresh of that import should resolve equivalency issue.
But the larger decision of whether to import FoodOn remains. I know the import originally stemmed from wanting to maintain the presence of the 600+ food terms that ENVO originally had and then gave to FoodOn (with FoodOn ids). But I don't see why people shouldn't just utilize FoodOn directly for food items, just as they go to NCBITaxon for organisms.

@wdduncan
Copy link
Member Author

@cmungall I updated the FOODON imports on my machine by running make imports/foodon_import.obo. This modified a number of files:

        modified:   imports/chebi_import.obo
	modified:   imports/chebi_import.owl
	modified:   imports/foodon_import.obo
	modified:   imports/foodon_import.owl
	modified:   imports/iao_import.obo
	modified:   imports/obi_import.obo
	modified:   imports/obi_import.owl
	modified:   imports/pato_import.obo
	modified:   imports/pato_import.owl
	modified:   imports/po_import.obo
	modified:   imports/po_import.owl

The number of files changed makes @ddooley and myself hesitant to push these changes to the repo. Also, we noticed that a number of FOODON classes were not properly placed in the ENVO hierarchy (see screenshot).
image

In any case, I ran make test using the new imports and the following errors were reported (comments added):

# fluid astronomical body part == compound astronomical body part
ERROR Equivalence: <http://purl.obolibrary.org/obo/ENVO_01001479> == <http://purl.obolibrary.org/obo/ENVO_01001784> 

# chebi:role == bfo:role
ERROR Equivalence: <http://purl.obolibrary.org/obo/CHEBI_50906> == <http://purl.obolibrary.org/obo/BFO_0000023>

Not sure what the best path forward is. One option is to relax the equivalent action check:

reason --reasoner ELK --equivalent-classes-allowed none --exclude-tautologies structural

merge the PR, and deal with the updates to FOODON in a separate PR.

@cmungall
Copy link
Member

we noticed that a number of FOODON classes were not properly placed in the ENVO hierarchy (see screenshot).

This is already a problem: https://www.ebi.ac.uk/ols/ontologies/envo

let's not try and solve all problems with this PR. Adding github actions and fixing inconsistencies will be a major step forward. Let's get this merged ASAP then improve imports

@cmungall
Copy link
Member

ERROR Equivalence: http://purl.obolibrary.org/obo/ENVO_01001479 == http://purl.obolibrary.org/obo/ENVO_01001784

didn't we solve this? Maybe a rebase would fix?

In any case, I agree with your strategy. Relax the constraints for now (we effectively have none now), then iterate

@wdduncan
Copy link
Member Author

@cmungall Re After relaxing the equivalent classes check and running make test , the following errors were reported:

ERROR	duplicate_label_synonym	garden	has_exact_synonym	garden
ERROR	duplicate_label_synonym	ocean	has_exact_synonym	ocean
ERROR	duplicate_label_synonym	sea	has_exact_synonym	sea
ERROR	duplicate_label_synonym	stream	has_exact_synonym	stream
ERROR	duplicate_label_synonym	reservoir	has_exact_synonym	reservoir

These duplicate labels are related to synonyms that have a database cross reference to the Geonames:feature. E.g.:
image

Do you want me to fix these or relax this check too?

@cmungall
Copy link
Member

cmungall commented Jan 21, 2022 via email

@wdduncan wdduncan marked this pull request as ready for review January 21, 2022 21:17
@wdduncan wdduncan merged commit f48f8ec into master Jan 21, 2022
@wdduncan wdduncan deleted the add-qc-workflow branch January 21, 2022 23:21
@wdduncan wdduncan mentioned this pull request Jan 28, 2022
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.

None yet

3 participants