-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat(analyzer): add recognizer-level threshold config #2116
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
base: main
Are you sure you want to change the base?
Changes from all commits
3c206f0
e458cd4
2c5cb34
08da64e
5de75b8
21606fc
94b050b
adac24d
2c49182
0bc3e87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,9 @@ recognizer_registry: | |
| - language: es | ||
| context: [tarjeta, credito, visa, mastercard, cc, amex, discover, jcb, diners, maestro, instapayment] | ||
| type: predefined | ||
| score_thresholds: | ||
| default: 0.4 | ||
| CREDIT_CARD: 0.7 | ||
|
|
||
| - name: DateRecognizer | ||
| supported_languages: | ||
|
|
@@ -119,6 +122,8 @@ recognizer_registry: | |
| """ | ||
| ``` | ||
|
|
||
| Each recognizer can set a `default` cutoff and entity-specific overrides in `score_thresholds`. An explicit request threshold takes priority, followed by an entity override, the recognizer default, and the analyzer's `default_score_threshold`. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an important implementation detail. If we pass
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here also, please use "threshold" instead of "cutoff" |
||
|
|
||
| ### NLP Engine parameters | ||
|
|
||
| ([default file](https://github.com/data-privacy-stack/presidio/blob/main/presidio-analyzer/presidio_analyzer/conf/default.yaml)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -25,6 +25,7 @@ | |||||
|
|
||||||
| REGEX_TIMEOUT_SECONDS = int(os.environ.get("REGEX_TIMEOUT_SECONDS", 60)) | ||||||
|
|
||||||
|
|
||||||
| class AnalyzerEngine: | ||||||
| """ | ||||||
| Entry point for Presidio Analyzer. | ||||||
|
|
@@ -254,9 +255,10 @@ def analyze( | |||||
| json.dumps([str(result.to_dict()) for result in results]), | ||||||
| ) | ||||||
|
|
||||||
| # Remove duplicates or low score results | ||||||
| # Filter low-score results before deduplication so recognizer-specific | ||||||
| # thresholds do not get lost when duplicate spans collapse. | ||||||
| results = self.__remove_low_scores(results, score_threshold, recognizers) | ||||||
| results = EntityRecognizer.remove_duplicates(results) | ||||||
| results = self.__remove_low_scores(results, score_threshold) | ||||||
|
|
||||||
| if allow_list: | ||||||
| results = self._remove_allow_list( | ||||||
|
|
@@ -330,21 +332,56 @@ def _enhance_using_context( | |||||
| return results | ||||||
|
|
||||||
| def __remove_low_scores( | ||||||
| self, results: List[RecognizerResult], score_threshold: float = None | ||||||
| self, | ||||||
| results: List[RecognizerResult], | ||||||
| score_threshold: float = None, | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| recognizers: Optional[List[EntityRecognizer]] = None, | ||||||
| ) -> List[RecognizerResult]: | ||||||
| """ | ||||||
| Remove results for which the confidence is lower than the threshold. | ||||||
|
|
||||||
| :param results: List of RecognizerResult | ||||||
| :param score_threshold: float value for minimum possible confidence | ||||||
| :param recognizers: recognizers selected for this request | ||||||
| :return: List[RecognizerResult] | ||||||
| """ | ||||||
| if score_threshold is None: | ||||||
| score_threshold = self.default_score_threshold | ||||||
| recognizers_by_id = { | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if |
||||||
| recognizer.id: recognizer for recognizer in recognizers or [] | ||||||
| } | ||||||
| return [ | ||||||
| result | ||||||
| for result in results | ||||||
| if result.score | ||||||
| >= self.__get_result_score_threshold(result, recognizers_by_id) | ||||||
| ] | ||||||
|
|
||||||
| new_results = [result for result in results if result.score >= score_threshold] | ||||||
| return new_results | ||||||
|
|
||||||
| def __get_result_score_threshold( | ||||||
| self, | ||||||
| result: RecognizerResult, | ||||||
| recognizers_by_id: dict, | ||||||
| ) -> float: | ||||||
| """Resolve the threshold to apply for a single recognizer result.""" | ||||||
| metadata = result.recognition_metadata or {} | ||||||
| recognizer_id = metadata.get(RecognizerResult.RECOGNIZER_IDENTIFIER_KEY) | ||||||
| recognizer = recognizers_by_id.get(recognizer_id) | ||||||
| if recognizer is None: | ||||||
| return self.default_score_threshold | ||||||
| recognizer_thresholds = recognizer.score_thresholds | ||||||
|
|
||||||
| entity_threshold = recognizer_thresholds.get(result.entity_type) | ||||||
| if entity_threshold is not None: | ||||||
| return entity_threshold | ||||||
|
|
||||||
| recognizer_default_threshold = recognizer_thresholds.get("default") | ||||||
| if recognizer_default_threshold is not None: | ||||||
| return recognizer_default_threshold | ||||||
|
|
||||||
| return self.default_score_threshold | ||||||
|
|
||||||
| @staticmethod | ||||||
| def _remove_allow_list( | ||||||
| results: List[RecognizerResult], | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| import logging | ||
| from abc import abstractmethod | ||
| from typing import TYPE_CHECKING, ClassVar, Dict, List, Optional, Tuple | ||
| from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, Optional, Tuple | ||
|
|
||
| from presidio_analyzer import RecognizerResult | ||
| from presidio_analyzer.score_thresholds import normalize_score_thresholds | ||
|
|
||
| if TYPE_CHECKING: | ||
| from presidio_analyzer.nlp_engine import NlpArtifacts | ||
|
|
@@ -33,6 +34,7 @@ class EntityRecognizer: | |
| recognizers may set it per instance; predefined recognizers should | ||
| prefer the class-level :attr:`COUNTRY_CODE`. Values are stripped, | ||
| lower-cased, and must match :attr:`COUNTRY_CODE` when both are set. | ||
| :param score_thresholds: Optional default and entity-specific score thresholds. | ||
| """ | ||
|
|
||
| MIN_SCORE = 0 | ||
|
|
@@ -53,6 +55,7 @@ def __init__( | |
| version: str = "0.0.1", | ||
| context: Optional[List[str]] = None, | ||
| country_code: Optional[str] = None, | ||
| score_thresholds: Any = None, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we provide a more specific type? I assume it's |
||
| ): | ||
| self.supported_entities = supported_entities | ||
|
|
||
|
|
@@ -67,13 +70,24 @@ def __init__( | |
| self.version = version | ||
| self.is_loaded = False | ||
| self.context = context if context else [] | ||
| self.score_thresholds = score_thresholds | ||
|
|
||
| self._country_code = self._resolve_country_code(country_code) | ||
|
|
||
| self.load() | ||
| logger.info("Loaded recognizer: %s", self.name) | ||
| self.is_loaded = True | ||
|
|
||
| @property | ||
| def score_thresholds(self) -> Dict[str, float]: | ||
| """Return a defensive copy of this recognizer's score thresholds.""" | ||
| return self._score_thresholds.copy() | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is copied on every result. Is it necessary? |
||
|
|
||
| @score_thresholds.setter | ||
| def score_thresholds(self, value: Any) -> None: | ||
| """Validate and store this recognizer's score thresholds.""" | ||
| self._score_thresholds = normalize_score_thresholds(value) | ||
|
|
||
| @classmethod | ||
| def _resolve_country_code(cls, passed: Optional[str]) -> Optional[str]: | ||
| """Reconcile a constructor-passed country code with the class attribute. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| """Validation helpers for recognizer score thresholds.""" | ||
|
|
||
| from collections.abc import Mapping | ||
| from typing import Any, Dict | ||
|
|
||
|
|
||
| def validate_score_threshold(threshold: Any) -> float: | ||
| """Validate a score threshold without coercing its input type.""" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please align the docstring format to the rest of the repo (add
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have to keep threshold as |
||
| if not isinstance(threshold, (int, float)) or isinstance(threshold, bool): | ||
| raise ValueError(f"Score threshold must be numeric, got: {threshold}") | ||
| if not 0.0 <= threshold <= 1.0: | ||
| raise ValueError( | ||
| f"Score threshold must be between 0.0 and 1.0, got: {threshold}" | ||
| ) | ||
| return threshold | ||
|
|
||
|
|
||
| def normalize_score_thresholds(score_thresholds: Any) -> Dict[str, float]: | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comments as above |
||
| """Validate and defensively copy one recognizer's score thresholds.""" | ||
| if score_thresholds is None: | ||
| return {} | ||
| if not isinstance(score_thresholds, Mapping): | ||
| raise ValueError("score_thresholds must be a mapping") | ||
|
|
||
| normalized = {} | ||
| for entity, threshold in score_thresholds.items(): | ||
| if not isinstance(entity, str) or not entity or entity.strip() != entity: | ||
| raise ValueError("score_thresholds keys must be non-empty strings") | ||
| normalized[entity] = validate_score_threshold(threshold) | ||
| return normalized | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use "threshold" instead of "cutoff" to keep the jargon similar to the rest of the code base