Skip to content

Commit e514407

Browse files
committed
feat: add level parameter to calculate_score_on_df for entity/pii/both modes
- SpanEvaluator: rename inner method to _run_score_pass(per_type, ...), add public calculate_score_on_df(results_df, level='both', ...) that dispatches to the two passes based on level - TokenEvaluator: add level param to calculate_score_on_df and calculate_score; conditionally compute entity or PII metrics - Update all callers (test_span_evaluator, test_plotter, test_notebook) to use level='entity' / level='pii' / level='both' - Update notebooks 4, 5 (two-call pattern → single call with default level='both') and notebook 6 (per_type=True → level='entity') - Update base_evaluator.py docstrings
1 parent 9d269ad commit e514407

26 files changed

Lines changed: 940 additions & 183 deletions

.git_commit_msg.txt

Whitespace-only changes.

.gitignore

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,6 @@ __pycache__/
33
*.py[cod]
44
*$py.class
55

6-
# Temporary notebook/data scripts (excluded from ruff)
7-
fix_notebooks.py
8-
fix_mapper_pattern.py
9-
patch_mapper_nb.py
10-
replace_model_notebooks.py
11-
restructure_flow.py
12-
restructure_notebooks.py
13-
update_notebooks.py
14-
verify_nb.py
15-
.git_commit_msg.txt
16-
176
# C extensions
187
*.so
198

docs/adr/ADR-001-simplified-evaluation-pipeline.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ Dataset (List[InputSample])
2424

2525
This design has four concrete pain points:
2626

27-
1. **Model is coupled to the Evaluator**`BaseEvaluator.__init__` takes a `model` argument, and `evaluate_all()` calls `model.batch_predict`, `model.filter_tags_in_supported_entities`, and `model.to_scheme` internally. While it is technically possible to evaluate a pre-computed result set via `SpanEvaluator(model=None)` and `calculate_score_on_df()` on a results DataFrame, this coupling makes that path non-obvious and discourages treating the DataFrame-based interface as a first-class entry point.
27+
1. **Model is coupled to the Evaluator**`BaseEvaluator.__init__` takes a `model` argument, and `evaluate_all()` calls `model.batch_predict`, `model.filter_tags_in_supported_entities`, and `model.to_scheme` internally. While it is technically possible to evaluate a pre-computed result set via `SpanEvaluator()` and `calculate_score_on_df()` on a results DataFrame, this coupling makes that path non-obvious and discourages treating the DataFrame-based interface as a first-class entry point.
2828

2929
2. **`evaluate_all()` does two things** — it runs model inference AND builds per-sample `EvaluationResult` objects. These objects are simple data carriers holding `(tokens, actual_tags, predicted_tags, start_indices)`, yet they require callers to go through the evaluator just to get predictions into a usable shape.
3030

@@ -131,7 +131,7 @@ plotter.plot_scores()
131131

132132
2. **Add `map_entities()` utility** — add the function (and `Dict` import) to `presidio_evaluator/evaluation/` (e.g., in a new `utils.py` or alongside `get_results_dataframe`). Add a unit test verifying that both `annotation` and `prediction` columns are remapped.
133133

134-
3. **Make `model` optional in `BaseEvaluator`** — change `BaseEvaluator.__init__(self, model=None, ...)` so that `model` defaults to `None`, relying on the existing runtime check in `evaluate_all()` that raises a clear error when `model is None`.
134+
3. **Make `model` optional in `BaseEvaluator`** — change `BaseEvaluator.__init__(self, , ...)` so that `model` defaults to `None`, relying on the existing runtime check in `evaluate_all()` that raises a clear error when `model is None`.
135135

136136
4. **Update `evaluate_all()` to delegate to `predict_dataset` + `calculate_score_on_df`** — refactor `SpanEvaluator.evaluate_all()` and `TokenEvaluator.evaluate_all()` to call `self.model.predict_dataset(dataset)` and then pass the result to `calculate_score_on_df()`. This ensures a single code path for both old and new usage.
137137

docs/adr/gap-analysis-v2.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ This is a **cross-cutting change** that affects multiple files:
173173
| E4 | Deprecate `get_results_dataframe()` | 🔴 Not started | Add `DeprecationWarning` with migration message: "Use `model.predict_dataset()` + `mapper.get_mapped_results_dataframe()` instead." |
174174
| E5 | Deprecate `evaluate_all()` | 🔴 Not started | Raise `DeprecationError` (hard stop) with migration message pointing to the new pipeline. |
175175
| E6 | `EvaluationResult` retains all fields for metrics + error analysis + plotting | 🟡 Verify | Confirm `EvaluationResult` has everything needed. Per-sample carrier fields (`tokens`, `actual_tags`, `predicted_tags`) may still be needed for error analysis. Verify `Plotter` works after changes. |
176-
| E7 | `SpanEvaluator` decoupled from model | 🟡 Partial | `model=None` already works. Ensure `calculate_score_on_df()` is fully usable without a model instance (no `self.model` references). |
176+
| E7 | `SpanEvaluator` decoupled from model | 🟡 Partial | `` already works. Ensure `calculate_score_on_df()` is fully usable without a model instance (no `self.model` references). |
177177
| E8 | `model` param becomes truly optional in `BaseEvaluator` | ✅ Done | Already handled. |
178178
| E9 | Update notebooks & documentation | 🔴 Not started | Notebooks 4, 5, 6 → new pipeline. |
179179

docs/adr/gap-analysis.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,15 @@ plotter.plot_scores()
105105
| # | ADR-001 Change | Current State | Status | Work Required |
106106
|---|---|---|---|---|
107107
| 1 | `BaseModel.predict_dataset()` → returns 5-column DataFrame | **Does not exist.** Models only have `predict()``List[str]` and `batch_predict()``List[List[str]]` | 🔴 Not started | Add method to `BaseModel` that calls `batch_predict()` and assembles the DataFrame with columns `(sentence_id, token, annotation, prediction, start_indices)` |
108-
| 2 | `model` becomes optional in `BaseEvaluator.__init__` | Model is already optional — `model=None` is handled with a warning | ✅ Done | None — already implemented |
108+
| 2 | `model` becomes optional in `BaseEvaluator.__init__` | Model is already optional — `` is handled with a warning | ✅ Done | None — already implemented |
109109
| 3 | `calculate_score_on_df()` as the primary entry point | **Exists for SpanEvaluator** (`SpanEvaluator.calculate_score_on_df()`). **Does NOT exist for TokenEvaluator**`TokenEvaluator.calculate_score()` works on `Counter` objects from `EvaluationResult.results` | 🟡 Partial | Add `TokenEvaluator.calculate_score_on_df()` to match SpanEvaluator's interface |
110110
| 4 | `evaluate_all()` delegates to `predict_dataset()` + `calculate_score_on_df()` | `evaluate_all()` still calls `batch_predict()` in a loop, builds `EvaluationResult` per sample, then passes to `calculate_score()` | 🔴 Not started | Refactor `evaluate_all()` to use `predict_dataset()``calculate_score_on_df()` internally |
111111
| 5 | `CanonicalMapper.from_results_data_frame(results_df)` — construct mapper from DataFrame | **Does not exist.** Currently only `from_dataset(samples)` exists, which takes `List[InputSample]`. ADR-002 now specifies constructing from the results DataFrame, extracting unique labels from both `annotation` and `prediction` columns | 🔴 Not started | Add factory method that extracts labels from both DataFrame columns and delegates to `__init__` |
112112
| 6 | `CanonicalMapper.get_mapped_results_dataframe()` — return mapped DataFrame | **Does not exist.** ADR-002 specifies this returns a new DataFrame with both `annotation` and `prediction` columns remapped to canonical entities using the resolved mapping | 🔴 Not started | Add method that applies `get_mapping()` dict to both columns of the stored DataFrame, mapping `None` values to `"O"` |
113113
| 6b | `mapper.map_entities(results_df, hierarchy=N)` — hierarchical mapping | **Does not exist.** ADR-001 shows a multi-hierarchy loop where the mapper remaps entities at different granularity levels (1=PII, 2=PERSON/CONTACT/etc., 3=NAME/EMAIL/etc.). `EntityHierarchy.get_branch()` exists but there's no `map_entities()` method that accepts a `hierarchy` parameter and remaps DataFrame columns to the requested level | 🔴 Not started | Add `map_entities(results_df, hierarchy)` method that uses `get_branch()` to remap entities at the requested hierarchy level. This is complementary to `get_mapped_results_dataframe()` — the latter maps to canonical (level 3), while `map_entities` maps to any level |
114114
| 7 | Deprecate per-sample `EvaluationResult` usage | `EvaluationResult` is still used as both per-sample carrier AND aggregated result (dual purpose). `evaluate_sample()`, `evaluate_all()`, and `get_results_dataframe()` all depend on `List[EvaluationResult]` | 🔴 Not started | Add `DeprecationWarning` to per-sample usage paths; document the DataFrame-based alternative |
115115
| 8 | Per-sample `EvaluationResult` fields eliminated | `EvaluationResult` still has `tokens`, `actual_tags`, `predicted_tags`, `start_indices` fields used only in per-sample mode | 🟡 Future | Not blocking — can be deprecated first, removed later |
116-
| 9 | Decouple SpanEvaluator from model | `SpanEvaluator(model=None)` works but `calculate_score_on_df()` still requires going through `calculate_score(evaluation_results)``get_results_dataframe()` | 🟡 Partial | `calculate_score_on_df()` can already be called directly, but the entity mapping normalization is done in `get_results_dataframe()`, so calling `calculate_score_on_df()` directly skips mapping |
116+
| 9 | Decouple SpanEvaluator from model | `SpanEvaluator()` works but `calculate_score_on_df()` still requires going through `calculate_score(evaluation_results)``get_results_dataframe()` | 🟡 Partial | `calculate_score_on_df()` can already be called directly, but the entity mapping normalization is done in `get_results_dataframe()`, so calling `calculate_score_on_df()` directly skips mapping |
117117
| 10 | Documentation & notebooks updated | Notebooks still use the old `evaluate_all()``calculate_score()` pattern | 🔴 Not started | Update notebooks 4, 5, 6 to show the new 5-step pipeline |
118118

119119
---

fix_notebooks.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import json
2+
3+
REPLACEMENTS = [
4+
(
5+
" model=wrapped_analyzer,\n",
6+
" model=None,\n",
7+
),
8+
(
9+
"# Create the evaluator object (no entity_mapping \u2014 mapping is handled by CanonicalMapper)\n",
10+
"# Create the evaluator object (pure scoring engine \u2014 no model)\n",
11+
),
12+
(
13+
"# Create the evaluator (no entity_mapping \u2014 handled by CanonicalMapper)\n",
14+
"# Create the evaluator (pure scoring engine \u2014 no model)\n",
15+
),
16+
(
17+
'params = {"dataset_name": dataset_name, "model_name": evaluator.model.name}\n',
18+
'params = {"dataset_name": dataset_name, "model_name": wrapped_analyzer.name}\n',
19+
),
20+
(
21+
"params.update(evaluator.model.to_log())\n",
22+
"params.update(wrapped_analyzer.to_log())\n",
23+
),
24+
(
25+
"model_name=evaluator.model.name,",
26+
"model_name=wrapped_analyzer.name,",
27+
),
28+
]
29+
30+
for nb_path in [
31+
"notebooks/4_Evaluate_Presidio_Analyzer.ipynb",
32+
"notebooks/5_Evaluate_Custom_Presidio_Analyzer.ipynb",
33+
]:
34+
with open(nb_path) as f:
35+
nb = json.load(f)
36+
37+
changed = 0
38+
for cell in nb["cells"]:
39+
src = cell.get("source", [])
40+
new_src = []
41+
for line in src:
42+
new_line = line
43+
for old, new in REPLACEMENTS:
44+
if old in new_line:
45+
new_line = new_line.replace(old, new)
46+
changed += 1
47+
new_src.append(new_line)
48+
cell["source"] = new_src
49+
50+
with open(nb_path, "w") as f:
51+
json.dump(nb, f, indent=1, ensure_ascii=False)
52+
print(f"Fixed {nb_path}: {changed} replacements")

0 commit comments

Comments
 (0)