Skip to content

fix(analyzer): make the shipped HuggingFaceNerRecognizer entry loadable - #2227

Open
Aryan-Pardeshi wants to merge 2 commits into
data-privacy-stack:mainfrom
Aryan-Pardeshi:fix/huggingface-recognizer-shipped-entry
Open

fix(analyzer): make the shipped HuggingFaceNerRecognizer entry loadable#2227
Aryan-Pardeshi wants to merge 2 commits into
data-privacy-stack:mainfrom
Aryan-Pardeshi:fix/huggingface-recognizer-shipped-entry

Conversation

@Aryan-Pardeshi

Copy link
Copy Markdown

Fixes #2222

HuggingFaceNerRecognizer ships in default_recognizers.yaml with enabled: false and no model_name. Flipping that switch does not work: EntityRecognizer.__init__ calls load(), load() raised when model_name was unset, and the error therefore arrived during registry construction rather than at analyze time. The shipped entry advertised a toggle that always crashed.

The issue offers three acceptable fixes. I took the deferred-load one:

  • load() now returns early with an info-level log when model_name is unset, so the registry builds.
  • analyze() raises a ValueError naming what is missing and where to set it, including default_recognizers.yaml, so the failure surfaces at the point of use with an actionable message.

I deliberately did not give the shipped entry a default model_name. Committing the project to a specific HuggingFace model reference in shipped config is a product decision rather than a bug fix, and it would make the loader test download a model. Happy to switch to that if you would rather — it is a small change on top of this one.

Because the entry now loads, HuggingFaceNerRecognizer has been removed from the NOT_LOADABLE_FROM_SHIPPED_ENTRY exclusion in test_recognizers_loader_utils.py, which #2170 added with a guard test to keep the gap visible. That set is now empty, and the recognizer is covered by the normal load test like every other entry. I left the comment in place explaining why it is empty, so the next person to need it knows what it was for.

The existing test asserting the old ValueError from load() is updated to assert the new analyze-time error instead, and a test covers registry construction succeeding with the recognizer instantiated and unloaded.

presidio-analyzer/tests/test_huggingface_ner_recognizer.py and test_recognizers_loader_utils.py: 338 passed, 3 skipped. ruff check presidio_analyzer tests reports all checks passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HuggingFaceNerRecognizer cannot be enabled from its shipped default_recognizers.yaml entry (no model_name, constructor raises)

1 participant