Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,25 @@ def load(self) -> None:
1. Hardware acceleration setup (CUDA validation and fallback)
2. Lazy-loading of the heavyweight ML pipeline.

:raises ValueError: If model_name is not set
Without ``model_name`` the pipeline cannot be built, and this returns
without one rather than raising: ``EntityRecognizer.__init__`` calls
``load()``, so raising here aborted the construction of the whole
registry for anyone who enabled the shipped
``default_recognizers.yaml`` entry, which carries no ``model_name``.
The recognizer is then registered but inactive, and ``analyze()``
raises with the same actionable message, so a missing model is still
reported rather than silently returning no entities.
"""
if self.ner_pipeline is not None:
return

if not self.model_name:
raise ValueError(
"model_name must be set before calling load(). "
"Pass it to __init__() or set it directly."
logger.warning(
"%s has no model_name and stays inactive. Set model_name to "
"use it, either in __init__() or on the recognizer entry.",
self.name,
)
return

# Device validation and fallback
device = self.device
Expand Down Expand Up @@ -430,6 +439,17 @@ def analyze(
# Defensive guard for entities input
entities = entities or []

if not self.model_name:
# load() leaves the recognizer inactive in this case, so that the
# shipped default_recognizers.yaml entry can be enabled without
# aborting the construction of the registry. Raising here rather
# than returning [] keeps a misconfigured recognizer from reading
# as "this text contains no PII".
raise ValueError(
"model_name must be set before calling analyze(). "
"Pass it to __init__() or set it directly."
)

if not self.ner_pipeline:
self.load()

Expand Down
8 changes: 6 additions & 2 deletions presidio-analyzer/tests/test_huggingface_ner_recognizer.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,14 @@ def test_hf_recognizer_load_errors():
with pytest.raises(ImportError):
HuggingFaceNerRecognizer(model_name="test")

# 2. Test ValueError when model_name is missing
# 2. Without model_name the recognizer builds but stays inactive, so that
# enabling the shipped default_recognizers.yaml entry does not abort the
# construction of the registry. The missing model is reported on use.
with patch(path, new=MagicMock()):
recognizer = HuggingFaceNerRecognizer(model_name=None)
assert recognizer.ner_pipeline is None
with pytest.raises(ValueError, match="model_name must be set"):
HuggingFaceNerRecognizer(model_name=None)
recognizer.analyze("Katherine lives in Seoul", entities=["PERSON"])


@pytest.mark.usefixtures("mock_torch_installed")
Expand Down
15 changes: 5 additions & 10 deletions presidio-analyzer/tests/test_recognizers_loader_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import inspect
import re
from pathlib import Path
from typing import Dict, List
from typing import Dict, List, Set

import presidio_analyzer.predefined_recognizers as predefined
import pytest
Expand Down Expand Up @@ -607,15 +607,10 @@ def test_yaml_country_code_blank_value_raises():
LOADER_KWARGS = ("name", "supported_language")

# Entries that cannot load from their shipped configuration even with every
# dependency installed, so the load test below cannot cover them.
#
# ``HuggingFaceNerRecognizer``: ``EntityRecognizer.__init__`` calls ``load()``
# unconditionally and ``load()`` requires ``model_name``, which the shipped
# entry does not supply -- it raises ValueError once ``transformers`` and
# ``torch`` are present. That is a pre-existing defect in the entry, not
# something this contract can assert away, and adding ``model_name`` here would
# make the test download a model. It stays covered by the resolve test.
NOT_LOADABLE_FROM_SHIPPED_ENTRY = {"HuggingFaceNerRecognizer"}
# dependency installed, so the load test below cannot cover them. Empty: an
# entry that cannot be loaded as shipped is a defect in the entry, and keeping
# the set makes the next one visible here instead of silently untested.
NOT_LOADABLE_FROM_SHIPPED_ENTRY: Set[str] = set()

# Entries gated behind an optional dependency, for which refusing to load with an
# actionable ImportError is the intended behavior. The skip is scoped to these
Expand Down