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

Support overriding registered functions in configs #12623

Merged

Conversation

adrianeboyd
Copy link
Contributor

Description

Support overriding registered functions in configs. Previously the registry name was parsed as a section name rather than as a registry name.

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.

@adrianeboyd adrianeboyd added bug Bugs and behaviour differing from documentation feat / config Feature: Training config labels May 11, 2023
@adrianeboyd
Copy link
Contributor Author

As reported in: #12618

@adrianeboyd adrianeboyd marked this pull request as draft May 11, 2023 12:16
@adrianeboyd
Copy link
Contributor Author

This still needs to address the CLI commands that call walk_dict.

@adrianeboyd
Copy link
Contributor Author

This is getting messy for nested things, let me think about this a bit more...

@adrianeboyd
Copy link
Contributor Author

adrianeboyd commented Jun 2, 2023

So the flag is kind of overkill (and can be renamed if anyone has any better ideas), but if anyone happened to be using walk_dict, then its default behavior doesn't change.

I think since overrides are applied before the config is resolved, it shouldn't be a problem to treat anything at or below the first registered function as a value, even if it contains nested registered functions.

@adrianeboyd adrianeboyd marked this pull request as ready for review June 2, 2023 09:54
@honnibal honnibal merged commit 65f6c9c into explosion:master Jun 27, 2023
13 checks passed
adrianeboyd added a commit to adrianeboyd/spaCy that referenced this pull request Jun 28, 2023
Support overriding registered functions in configs. Previously the registry name was parsed as a section name rather than as a registry name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs and behaviour differing from documentation feat / config Feature: Training config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants