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

Get rid of partof cycles #314

Merged
merged 10 commits into from
May 10, 2024
Merged

Get rid of partof cycles #314

merged 10 commits into from
May 10, 2024

Conversation

matentzn
Copy link
Contributor

@matentzn matentzn commented Feb 19, 2024

Fixes #313.

This PR ensures that we never release OBA again with subclass of cycles (as a consequence of relaxing owl:equivalentClasses).

Depends on

@rays22 can you take this PR over? It is basically done. Remaining tasks:

  • Help Arwa fix the above and merge into branch
  • Run a test release on the branch and convince yourself I did not break anything major (looking at oba.owl in Protege, maybe look at the diff - whatever you feel is necessary).

@@ -81,12 +81,15 @@ $(MIRRORDIR)/lipidmaps.owl: $(TEMPLATEDIR)/lipidmaps.tsv
$(ONT)-full.owl: $(SRC) $(OTHER_SRC)
echo "INFO: Running FULL release, which is customised for OBA."
$(ROBOT) merge --input $< \
merge -i components/reasoner_axioms.owl \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is removing the following axiom:

ReflexiveObjectProperty(<http://purl.obolibrary.org/obo/BFO_0000050>)

Due to the global properties of this statement, we are dropping it.

@@ -81,12 +81,15 @@ $(MIRRORDIR)/lipidmaps.owl: $(TEMPLATEDIR)/lipidmaps.tsv
$(ONT)-full.owl: $(SRC) $(OTHER_SRC)
echo "INFO: Running FULL release, which is customised for OBA."
$(ROBOT) merge --input $< \
merge -i components/reasoner_axioms.owl \
materialize -T basic_properties.txt \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do this a bit later now.

reason --reasoner ELK --equivalent-classes-allowed asserted-only --exclude-tautologies structural \
remove --term PR:000000001 \
--term SO:0000252 \
--term SO:0000234 \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We remove a number of asserted equivalent classes forcefully. All three terms have equivalent classes that we are preserving.

remove --term PR:000000001 \
--term SO:0000252 \
--term SO:0000234 \
reason --reasoner ELK --equivalent-classes-allowed none --exclude-tautologies structural \
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only for QC purposes.

Copy link
Contributor

@rays22 rays22 left a comment

Choose a reason for hiding this comment

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

  1. I have created two mock releases with or without the changes, respectively, in this PR.
    oba.owl with the changes contains a lot more axioms of the type
- SubClassOf(<http://purl.obolibrary.org/obo/XX_12345678> ObjectSomeValuesFrom(<http://purl.obolibrary.org/obo/BFO_0000050> <...>)

that are related to part of something.
2. I have also checked the oba.owl release products in Protege carefully looking for major differences or problems in the two versions, but other than the extra part of-type subclasses (which make sense) I could not find anything outstanding.

🎉

@matentzn
Copy link
Contributor Author

THANKS! Feel free to merge, and when all other PRs are processed, create a release! Thank you so much for checking!

@rays22 rays22 merged commit 6ca0354 into master May 10, 2024
1 check passed
@rays22 rays22 deleted the part-of-cycles branch May 10, 2024 17:06
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.

PR:000000001 and protein (CHEBI:36080) has cyclic dependency
2 participants