-
Notifications
You must be signed in to change notification settings - Fork 30
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
Fixes all duplicate scoped synonym #2332
Conversation
converting to a draft so it doesn't accidentally get merged - this needs to be reviewed before merging. I'll post a diff here to make reviewing easier. |
@anitacaron can you check in the update queries(src/sparql/update)? We should do that to make it easier to do similar things in the future. Also, I think we should probably split both problems into separate update queries.. I think you don't capture case 1 entirely because you only look at terms that have at least two synonyms? |
@matentzn I used two queries to make the changes. I put here the two mixed because the difference is a few lines. |
@shawntanzk dont worry about that, we can use normalisation for contracting these duplicate synonyms! No need to change the SPARQL I think. |
Great job! Assuming this is done? |
I think the only thing that can be reviewed is wether the synonyms are all exact. Many are not. But this is not really a hill you want to die on here. It will take dozens of curation hours to fix something that has been like this for 15 years, and no one complained so far. I would not get bogged down by details and just focus on QC and infrastructure so we can actually ensure that stuff that is added moving forward is not broken. |
@shawntanzk can we merge this? Seems a waste to let this open for people to never review this.. |
honestly not super sure, would rather bring this up in a call - people had quite strong opinions about this last time it was discussed |
|
I'm closing this PR as it's already too old, and it's better to have a new, clean PR. I've linked the queries used here in the issue. |
Two cases were fixed, when:
The cases were fixed to have
oboInOwl:hasExactSynonym
.The following sparql update query were used (here it's mixed the fixes for the two cases):
Fixes #2305