fix: let the shipped HuggingFaceNerRecognizer entry be enabled (#2222) - #2228
Open
overgoy wants to merge 1 commit into
Open
fix: let the shipped HuggingFaceNerRecognizer entry be enabled (#2222)#2228overgoy wants to merge 1 commit into
overgoy wants to merge 1 commit into
Conversation
…privacy-stack#2222) default_recognizers.yaml ships HuggingFaceNerRecognizer with enabled: false and no model_name. EntityRecognizer.__init__ calls load(), and load() raised when model_name was missing, so flipping the shipped entry to true aborted the construction of the whole registry rather than adding one recognizer. The switch existed but could never be turned on. load() now leaves the recognizer inactive and logs why, so the registry builds. analyze() raises with the same actionable message, so a recognizer without a model is still reported rather than quietly returning no entities, which for a PII library would read as "this text is clean". The guard in analyze() is on model_name rather than on a missing pipeline, so lazy loading through load() keeps working as before. HuggingFaceNerRecognizer therefore leaves NOT_LOADABLE_FROM_SHIPPED_ENTRY, which the contract test kept it in with a comment calling it a pre-existing defect in the entry. The set stays in place, and empty, so the next entry that cannot load as shipped is visible there instead of silently untested.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enabling the shipped
HuggingFaceNerRecognizerentry indefault_recognizers.yamlcrashes the whole registry build, because the entry carries nomodel_nameandEntityRecognizer.__init__callsload(). The switch exists but can never be turned on.load()now leaves the recognizer inactive and logs why, so the registry builds.analyze()raises with the same actionable message, so a recognizer without a model is reported rather than quietly returning no entities, which for a PII library would read as "this text is clean".The guard in
analyze()is onmodel_namerather than on a missing pipeline, so lazy loading throughload()behaves exactly as before.HuggingFaceNerRecognizercan therefore leaveNOT_LOADABLE_FROM_SHIPPED_ENTRY, where the contract test kept it with a comment calling it a pre-existing defect in the entry. The set stays in place, and empty, so the next entry that cannot load as shipped is visible there instead of silently untested.Fixes #2222.
Verified locally: the reproduction from the issue now builds a registry containing the recognizer, and using it without a model still explains why. Full analyzer suite passes (3316 passed, 13 skipped);
ruff checkis clean.ruff formatreports one pre-existing complaint intests/test_recognizers_loader_utils.pythat is unrelated to these lines, so I left it alone rather than reformatting untouched code.Disclosure: this was prepared with an AI coding assistant. The project's full analyzer test suite and linter were run locally before opening this.