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

Language.replace_listeners: Pass the replaced listener and the tok2vec pipe to the callback #12785

Merged

Conversation

shadeMe
Copy link
Contributor

@shadeMe shadeMe commented Jul 4, 2023

Description

Prior to this PR, Language.replace_listeners only passed the copied tok2vec/transformer model to the replace_listener callback. This worked for spacy-transformers as the library only uses a single TransformerListener class, i.e., the replacement model was always the same. However, spacy-curated-transformers introduces the concept of different listener classes for downstream components. So, the replacement process needs to provide extra information about the listener model being replaced to correctly select the target model.

This PR adds support for such situations by passing the following extra arguments to the replace_listener callback: the replaced listener instance and the tok2vec pipe whose model is copied into the downstream component. These argumentss are only passed when the callback accepts them, so the implementation is backwards-compatible with the callback used in spacy-transformers.

Types of change

Enhancement

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.

@shadeMe shadeMe added enhancement Feature requests and improvements feat / tok2vec Feature: Token-to-vector layer and pretraining feat / transformer Feature: Transformer v3.7 Related to v3.7 labels Jul 4, 2023
spacy/language.py Outdated Show resolved Hide resolved
extra/DEVELOPER_DOCS/Listeners.md Outdated Show resolved Hide resolved
Copy link
Member

@svlandeg svlandeg 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!

There's nowhere in the public docs that we need to update, right?

extra/DEVELOPER_DOCS/Listeners.md Outdated Show resolved Hide resolved
spacy/language.py Outdated Show resolved Hide resolved
extra/DEVELOPER_DOCS/Listeners.md Outdated Show resolved Hide resolved
@shadeMe
Copy link
Contributor Author

shadeMe commented Jul 5, 2023

There's nowhere in the public docs that we need to update, right?

No, I don't believe so.

@svlandeg svlandeg merged commit 8113cfb into explosion:develop Jul 5, 2023
9 checks passed
@shadeMe shadeMe deleted the feature/replace-listener-metadata branch July 5, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests and improvements feat / tok2vec Feature: Token-to-vector layer and pretraining feat / transformer Feature: Transformer v3.7 Related to v3.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants