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.

Sign up for GitHub

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

Jump to bottom

EditTreeLemmatizer: correctly add strings when initializing from labels #11934

Merged
merged 1 commit into from
Dec 7, 2022
Merged

EditTreeLemmatizer: correctly add strings when initializing from labels #11934

merged 1 commit into from
Dec 7, 2022

Conversation

danieldk
Copy link
Contributor

@danieldk danieldk commented Dec 5, 2022 •
edited by polm
Loading

Description

Strings in replacement nodes where not added to the StringStore when EditTreeLemmatizer was initialized from a set of labels. The corresponding test did not capture this because it added the strings through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing' of the bug in the test.

Bug found while implementing distillation, since it relies purely on this functionality to initialize the teacher.

Types of change

Bug fix.

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the tests, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@danieldk
EditTreeLemmatizer: correctly add strings when initializing from labels
574bcc7 
Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
@danieldk danieldk added bug Bugs and behaviour differing from documentation feat / lemmatizer Feature: Rule-based and lookup lemmatization labels Dec 5, 2022
polm
polm approved these changes Dec 7, 2022
View reviewed changes
Copy link
Contributor

@polm polm left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@polm polm merged commit 27fac7d into explosion:master Dec 7, 2022
adrianeboyd pushed a commit to adrianeboyd/spaCy that referenced this pull request Dec 9, 2022
@danieldk @adrianeboyd
EditTreeLemmatizer: correctly add strings when initializing from labe…
c4e5bc5 
…ls (explosion#11934)

Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
adrianeboyd pushed a commit to adrianeboyd/spaCy that referenced this pull request Dec 9, 2022
@danieldk @adrianeboyd
EditTreeLemmatizer: correctly add strings when initializing from labe…
e3e8050 
…ls (explosion#11934)

Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
adrianeboyd pushed a commit to adrianeboyd/spaCy that referenced this pull request Dec 9, 2022
@danieldk @adrianeboyd
EditTreeLemmatizer: correctly add strings when initializing from labe…
26fb9d9 
…ls (explosion#11934)

Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
adrianeboyd pushed a commit to adrianeboyd/spaCy that referenced this pull request Dec 9, 2022
@danieldk @adrianeboyd
EditTreeLemmatizer: correctly add strings when initializing from labe…
8657539 
…ls (explosion#11934)

Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
adrianeboyd pushed a commit to adrianeboyd/spaCy that referenced this pull request Dec 9, 2022
@danieldk @adrianeboyd
EditTreeLemmatizer: correctly add strings when initializing from labe…
1df507c 
…ls (explosion#11934)

Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
adrianeboyd pushed a commit to adrianeboyd/spaCy that referenced this pull request Dec 9, 2022
@danieldk @adrianeboyd
EditTreeLemmatizer: correctly add strings when initializing from labe…
809887a 
…ls (explosion#11934)

Strings in replacement nodes where not added to the `StringStore`
when `EditTreeLemmatizer` was initialized from a set of labels. The
corresponding test did not capture this because it added the strings
through the examples that were passed to the initialization.

This change fixes both this bug in the initialization as the 'shadowing'
of the bug in the test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@polm polm polm approved these changes

Assignees
No one assigned
Labels
bug Bugs and behaviour differing from documentation feat / lemmatizer Feature: Rule-based and lookup lemmatization
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@danieldk @polm