Skip to content

fix(privacy): missing values stay missing, categorical k-anonymity, duplicate labels, fpe audit metadata, NER status - #399

Merged
kevincostner17 merged 5 commits into
mainfrom
fix/privacy-missing-values-kanon-labels
Sep 15, 2026
Merged

kevincostner17 merged 5 commits into
mainfrom
fix/privacy-missing-values-kanon-labels

Conversation

@kevincostner17

Copy link
Copy Markdown
Contributor

Summary

One commit per issue.

#243: keep pd.NA and NaT cells missing when masking

anonymize (rule and detection paths), detect_pii and apply_privacy_policy treated a cell as missing only when it was None or a float NaN. pd.NA (nullable string/Int64/boolean) and NaT were stringified to "<NA>"/"NaT" and masked like values, and cells_changed counted them. A shared _is_missing_scalar() check now passes those cells through before masking. The masking functions are unchanged, so output for non-null cells is identical.

#244: categorical quasi-identifiers in check_k_anonymity

groupby(..., dropna=False).size() with categorical keys emitted every category combination, including empty ones. Those size-0 groups set smallest_class_size=0 and failed the check. Categorical keys are now grouped as object values and size-0 groups are dropped. observed=True is avoided because of pandas 1.5 edge cases with dropna=False. clean_enterprise with KAnonymityConfig benefits too.

#265 (part 1): duplicate column labels in detect_pii / anonymize

A duplicated label made frame[col] return a DataFrame, which crashed with AttributeError. detect_pii now raises a ValueError naming the duplicated labels. anonymize raises when detection is enabled or when a rule targets a duplicated label. Rules on unique columns still work. The other parts of #265 were fixed in #381 and #388.

#281 (parts 1-2): fpe audit metadata

  • MaskingEvent.reversible now reflects what each cell actually used: tokenize, or fpe in crypto_fpe mode, and only when the rule sets reversible=True. The surrogate fallback is reported as not reversible.
  • metadata["fpe_mode"] is unchanged when a single mode was used. Mixed modes report "mixed" with metadata["fpe_modes"] = {column: {mode: count}}.
  • Masked values are unchanged.

#282: NER metadata when the Presidio analyzer cannot start

A failed AnalyzerEngine() start-up was swallowed, retried for every cell, and still reported as ner=True. The failure is now cached, and detect_pii emits one UserWarning and skips the NER pass. The metadata reports ner, ner_requested, ner_active and ner_error. The PIIDetectionConfig docstring is updated.

Behaviour changes: cells_changed no longer counts NA/NaT cells; duplicate-label frames raise ValueError instead of AttributeError; metadata["ner"] is False when NER could not run; a failed Presidio start-up is not retried within the process.

Tests

New tests/test_privacy_missing_and_labels.py:

Out of scope, found while testing: the quarantine policy action raises TypeError on Int64/boolean columns (Series.where with a string placeholder). Those two combinations are excluded from the #243 test and should be tracked separately.

Verification

  • ruff check .: passed; mypy src/freshdata: no issues
  • pytest -m "not online and not large" on the branch (base 2d098cb): Python 3.12 / pandas 2.3.3: 5219 passed, 13 skipped; Python 3.9 / pandas 1.5.3: 5215 passed, 17 skipped
  • After rebasing onto main 1ac83fc, together with the other open fix branches: Python 3.12: 5532 passed, 13 skipped; Python 3.9: 5528 passed, 17 skipped

Closes #243
Closes #244
Closes #282
Refs #281
Refs #265

The rule path and detection path of anonymize(), detect_pii() and
apply_privacy_policy() treated a cell as missing only when it was None
or a float NaN. pd.NA (nullable string, Int64 and boolean columns) and
NaT fell through that check, were stringified to "<NA>"/"NaT" and then
redacted, tokenized or pseudonymised like real values. Missing cells
came back as placeholders or tokens and cells_changed counted them.

Add _is_missing_scalar(), which accepts None and any scalar that
pd.isna() reports as missing, and use it for the four null checks.
Missing cells are now passed through unchanged before masking; the
masking functions themselves are unchanged, so non-null output is
identical to before.

Tests cover string, Int64, boolean, datetime64[ns] and object columns
holding pd.NA across every rule strategy, the tokenize, pseudonymize,
redact and quarantine policy actions, the detection path and the three
reproductions from the issue.

Closes #243
check_k_anonymity grouped rows with groupby(..., dropna=False).size()
and the default observed=False. For categorical quasi-identifiers pandas
then emits every combination of categories, including combinations with
no rows. Those size-0 groups set smallest_class_size to 0, inflated
n_equivalence_classes, turned ok to False and were listed in
high_risk_groups, while the same data as object dtype passed. The same
report feeds clean_enterprise with KAnonymityConfig.

Group on the quasi-identifier columns with categorical columns cast to
object, so only observed value combinations form groups, and drop any
size-0 group. observed=True is not used because it mishandles
dropna=False on pandas 1.5. Missing quasi-identifier values still form
their own class.

Tests cover the reproduction, a categorical quasi-identifier holding
NaN, unused categories, mixed categorical and object quasi-identifiers,
and clean_enterprise with KAnonymityConfig.

Closes #244
detect_pii and the detection pass of anonymize loop over frame.columns
and read frame[col].dtype. For a duplicated label frame[col] returns a
DataFrame rather than a Series, so both functions failed with an
unhelpful AttributeError. A masking rule that selected a duplicated
label had the same problem, and _resolve_columns listed the label once
per occurrence.

Raise a ValueError naming the duplicated labels instead:

- detect_pii raises up front when any label is duplicated.
- anonymize raises when detection is enabled and any label is
  duplicated, or when a rule resolves to a duplicated label. The check
  runs before any rule is applied.

A rule that targets a unique column still works on a frame that has
duplicate labels elsewhere, as long as detection is not enabled.

Tests cover both reproductions, a rule aimed at a duplicated label, and
rules on a unique column next to duplicated ones with detection absent
or disabled.

Refs #265
anonymize() computed MaskingEvent.reversible from the rule alone, so an
fpe rule with reversible=True marked every event reversible even when
pyffx was unavailable and the cell was masked with the one-way surrogate
fallback. metadata["fpe_mode"] was also overwritten per cell, so a
column mixing real FPE and the surrogate fallback reported only the
mode of the last cell processed.

_apply_rule_column now sets each event's reversible flag from the mode
_mask_one returned for that cell: true only for tokenize, or for fpe
when the cell used crypto_fpe, and only when the rule asked for it. It
returns per-mode cell counts instead of the last mode. anonymize()
aggregates those counts: a single mode is still reported as the plain
fpe_mode string, and more than one mode is reported as
fpe_mode="mixed" with fpe_modes={column: {mode: count}}. Masked values
are unchanged.

Tests use pyffx=None and a stub pyffx that cannot encrypt some values to
cover the fallback, mixed modes within a column and across columns,
reversible tokenize, and a single-mode report that matches the previous
output.

Refs #281
detect_pii(config=PIIDetectionConfig(use_ner=True)) wrote
metadata={"ner": True} from the config flag alone. When
presidio_analyzer was importable but AnalyzerEngine() raised (for
example a missing language model), _get_presidio_analyzer swallowed the
exception without caching it. The NER pass then contributed nothing,
the report still claimed NER ran, no warning was emitted, and the
engine constructor was retried for every cell.

_get_presidio_analyzer now records the first failure as
"ExceptionType: message" in _PRESIDIO_ERROR and does not retry it.
detect_pii resolves the analyzer once per call when use_ner is set,
skips the per-cell NER pass when it is unavailable and emits one
UserWarning naming the error. The metadata reports the real state:
ner (now the active flag), ner_requested, ner_active, and ner_error
when NER was requested but could not run. The PIIDetectionConfig
docstring no longer says the pass is skipped silently.

Tests stub presidio_analyzer through sys.modules: an engine that fails
to start (one warning, correct metadata, constructor called once across
cells and calls), a working engine returning no results (ner_active
True), a missing package (import error recorded), and NER not requested.

Closes #282
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: bd3c72c4-82db-45f8-9137-958176185286


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

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

Labels

None yet

Projects

None yet

1 participant