-
Notifications
You must be signed in to change notification settings - Fork 74
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 how Template adds entities to the QuotedEntityChecker #1104
Conversation
It's not really a cycle but a sequence path. |
I think the problem is that: If there is no labels column, Template.addLabels() never adds anything to the |
Thank you, Nico! You said in Slack that adding column |
Thanks for confirming. It's still important for us to get to the bottom of this! |
Please reopen if you still want this PR. |
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.
@jamesaoverton looked through your changes, made some small comments for sanity checking but most likely everything is correct.
(Updated by @jamesaoverton)
Resolves #1133, resolves #1156.
docs/
have been added/updatedmvn verify
says all tests passmvn site
says all JavaDocs correctCHANGELOG.md
has been updatedReplaces private
Template.addLabels()
withTemplate.addEntities()
: for each row in the template, make sure to add an entity to the QuotedEntityChecker (QEC). The previous version skipped rows without a LABEL. The new version adds both the label (if present) and the ID to the QEC. I believe this is backwards compatible.I had problems with CI that seem unrelated, so I also dropped a test for subset extraction. See #1166.