-
Notifications
You must be signed in to change notification settings - Fork 54
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 subclass of sync #8431
Fix subclass of sync #8431
Conversation
Previously the syncing was wrong as the method was deleting _all_ subclass of axioms, regardless of whether they were existential restrictions or subclass between names. It took a while to find this solution, but I think it should work: 1. delete _all_ subclass of axioms after 2. backing up all subclass of axioms with existentials 3. backing uo all subclass of axioms without existentials, and then removing evidence from all the mondo ingest primary sources from it.
@matentzn does this mean that #8422 should not be merged? The results of running the Synonym Sync pipeline before this change? cc: @sabrinatoro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me based on what we discussed. However, I did not run this goal locally to test.
I am on the fence - the diff was so large on the data load #8432 that we cannot with all confidence say that, once the bug is fixed, no other issue emerges.. I am fine merging it, but I am also fine waiting, as we are not going to run subclass sync again until the bug is fixed |
Ok, sounds like we should merge it then to clear out PRs and make any future updates as needed once the bug in generating the subclass sync data is sorted since as you say the subclass sync will not be run otherwise. |
Previously the syncing was wrong as the method was deleting all subclass of axioms, regardless of whether they were existential restrictions or subclass between names. It took a while to find this solution, but I think it should work:
I accidentally merged the synonym sync commit with the URL change but this will disappear since #8430 is merged.