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

Fix repair merge axiom annotations #1223

Merged
merged 3 commits into from
Nov 12, 2024

Conversation

matentzn
Copy link
Contributor

@matentzn matentzn commented Nov 11, 2024

For some reason, ROBOT repair --merge-axiom-annotations not only was restricted to annotation axioms (which is wrong), it also drops logical axioms with annotations as they are (1) first added to the heap of axioms that need to be processed, (2) dropped from the ontology because they are in that heap and finally (3) because of the wrong conditional we are fixing here, not added back.

  • docs/ have been added/updated
  • tests have been added/updated
  • mvn verify says all tests pass
  • mvn site says all JavaDocs correct
  • CHANGELOG.md has been updated

This is so the diff will be easier to read for the extended repair check I intend to add in the next commit.
For some reason, ROBOT repair --merge-axiom-annotations not only was restricted to annotation axioms (which is wrong), it also drops logical axioms with annotations as they are (1) first added to the heap of axioms that need to be processed, (2) dropped from the ontology because they are in that heap and finally (3) because of the wrong conditional we are fixing here, not added back.

Adding and example for axiom annotation merging on subclass axioms to our test case
@matentzn
Copy link
Contributor Author

@jamesaoverton to review this more easily, @gouttegd taught me a great trick: review the commits one by one. This way, you don't see the big normalisation diffs in the test cases, and you only have to review 2 lines of code and about 10 lines of test cases.

@jamesaoverton jamesaoverton merged commit 82daac6 into master Nov 12, 2024
3 checks passed
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.

2 participants