From f6d13101690e2ce25f8fe42f22bbdf7cdbbb9c47 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:44:29 +0530 Subject: [PATCH] fix(explain): support non-string column labels and reject duplicate labels in explain and infer_roles explain_clean keyed before_stats/after_stats by str(label) but cell_changes by the raw label, and the HTML renderer looked up stats with the raw label. For integer labels (read_csv(header=None)) to_frame() reported 0 changed cells and the HTML view showed blank dtypes; tuple MultiIndex keys made to_dict()/to_html() fail in json.dumps, so _repr_html_() returned None. Every per-column mapping in ExplainReport is now keyed by str(label), to_frame()/to_dict() stringify keys defensively, and render_explain looks stats up by str(label). Narratives now match actions (which record str(label)), so integer-labelled columns are no longer silently omitted. explain_clean and infer_roles sorted per-column contexts by raw label, raising TypeError on mixed int/str labels. Both now sort by str(label). infer_roles keeps the original label in its "column" output so it still indexes the frame, passes str(label) as the semantic-type name, and accepts semantic_context column hints keyed by either form. Duplicate column labels made df[col] return a DataFrame and crashed both functions with AttributeError/TypeError. They now raise a clear ValueError ("... requires unique column labels; duplicated: [...]"), matching fill_missing. explain_clean also raises a ValueError naming labels whose string forms collide (1 and "1") rather than silently merging them in its str-keyed report; infer_roles, which keeps original labels, accepts them. Refs #232 (parts 3, 6) Refs #265 (part 3) --- src/freshdata/api.py | 16 ++- src/freshdata/explain.py | 55 +++++++--- src/freshdata/render/renderers.py | 5 +- tests/test_explain_column_labels.py | 162 ++++++++++++++++++++++++++++ 4 files changed, 221 insertions(+), 17 deletions(-) create mode 100644 tests/test_explain_column_labels.py diff --git a/src/freshdata/api.py b/src/freshdata/api.py index e4e5d17..b03e501 100644 --- a/src/freshdata/api.py +++ b/src/freshdata/api.py @@ -1123,6 +1123,13 @@ def _engine_mode(cfg: CleanConfig) -> EngineMode: return "balanced" if mode == "balanced" else "aggressive" +def _require_unique_labels(frame: pd.DataFrame, func: str) -> None: + """Reject duplicate column labels, which make ``frame[col]`` a DataFrame.""" + if not frame.columns.is_unique: + duplicated = sorted({str(c) for c in frame.columns[frame.columns.duplicated()]}) + raise ValueError(f"{func} requires unique column labels; duplicated: {duplicated}") + + def infer_roles( df: pd.DataFrame, *, @@ -1142,21 +1149,24 @@ def infer_roles( cfg = merge_options(config, strategy=strategy, **options) frame = to_pandas(df) + _require_unique_labels(frame, "infer_roles") contexts = build_contexts(frame, cfg) mode = _engine_mode(cfg) hints = cfg.semantic_context if isinstance(cfg.semantic_context, dict) else {} column_hints = hints.get("columns", {}) if isinstance(hints.get("columns"), dict) else {} rows = [] - for col, ctx in sorted(contexts.items()): + # Rows keep the original label (so ``frame[row["column"]]`` round-trips); + # ordering uses its string form so mixed int/str labels are comparable. + for col, ctx in sorted(contexts.items(), key=lambda kv: str(kv[0])): primary = None if ctx.missing_ratio > 0: primary = rank_missing_models(frame, col, ctx, cfg, mode=mode).primary hint = None - col_hint = column_hints.get(col) + col_hint = column_hints[col] if col in column_hints else column_hints.get(str(col)) if isinstance(col_hint, dict): hint = col_hint.get("semantic_type") inferred = infer_semantic_type( - col, + str(col), frame[col], role=ctx.role, hint=str(hint) if hint else None, diff --git a/src/freshdata/explain.py b/src/freshdata/explain.py index d705057..5cf07df 100644 --- a/src/freshdata/explain.py +++ b/src/freshdata/explain.py @@ -10,7 +10,7 @@ from pandas.api.types import is_numeric_dtype from .adapters.polars import to_pandas -from .api import infer_roles +from .api import _require_unique_labels, infer_roles from .cleaner import run_pipeline from .config import CleanConfig, merge_options from .engine.context import build_contexts @@ -18,6 +18,27 @@ from .report import Action, CleanReport +def _str_keys(mapping: dict[Any, Any]) -> dict[str, Any]: + return {str(k): v for k, v in mapping.items()} + + +def _require_distinct_label_names(df: pd.DataFrame, func: str) -> None: + """Reject distinct labels sharing a string form (e.g. ``1`` and ``"1"``). + + The report keys every per-column mapping by ``str(label)``, so such labels + would silently merge into one entry. + """ + by_name: dict[str, list[object]] = defaultdict(list) + for col in df.columns: + by_name[str(col)].append(col) + colliding = [labels for labels in by_name.values() if len(labels) > 1] + if colliding: + raise ValueError( + f"{func} requires column labels with distinct string forms; " + f"colliding: {colliding}" + ) + + def _column_stats(df: pd.DataFrame) -> dict[str, dict[str, Any]]: stats: dict[str, dict[str, Any]] = {} for col in df.columns: @@ -47,6 +68,8 @@ def _cell_changes(before: pd.DataFrame, after: pd.DataFrame) -> dict[str, int]: mark untouched values as changed. If either index carries duplicate labels, alignment is ambiguous and the conservative whole-column count is kept. Columns that exist only in ``after`` count all their cells. + + Keys are ``str(label)``, matching :func:`_column_stats`. """ changes: dict[str, int] = {} shared = [c for c in after.columns if c in before.columns] @@ -58,7 +81,7 @@ def _cell_changes(before: pd.DataFrame, after: pd.DataFrame) -> dict[str, int]: aligned_after = after.loc[common] else: for col in after.columns: - changes[col] = len(after) + changes[str(col)] = len(after) return changes for col in shared: left = aligned_before[col] @@ -77,10 +100,10 @@ def _cell_changes(before: pd.DataFrame, after: pd.DataFrame) -> dict[str, int]: if pd.isna(a) != pd.isna(b) or (not pd.isna(a) and not pd.isna(b) and a != b) ) - changes[col] = changed + changes[str(col)] = changed for col in after.columns: if col not in before.columns: - changes[col] = len(after) + changes[str(col)] = len(after) return changes @@ -97,7 +120,9 @@ def _narratives( if action.column: by_col[action.column].append(action) - for col, ctx in sorted(contexts.items()): + # Contexts are keyed by the original label; actions record ``str(label)``. + for label, ctx in sorted(contexts.items(), key=lambda kv: str(kv[0])): + col = str(label) col_actions = by_col.get(col, []) engine_actions = [a for a in col_actions if a.rationale] if engine_actions: @@ -173,14 +198,17 @@ def summary(self) -> str: def to_frame(self) -> pd.DataFrame: """One row per column: before/after dtype and changed-cell count.""" rows = [] - for col in sorted(set(self.before_stats) | set(self.after_stats)): - before = self.before_stats.get(col, {}) - after = self.after_stats.get(col, {}) + before_stats = _str_keys(self.before_stats) + after_stats = _str_keys(self.after_stats) + cell_changes = _str_keys(self.cell_changes) + for col in sorted(set(before_stats) | set(after_stats)): + before = before_stats.get(col, {}) + after = after_stats.get(col, {}) rows.append({ "column": col, "before_dtype": before.get("dtype"), "after_dtype": after.get("dtype"), - "changed_cells": self.cell_changes.get(col, 0), + "changed_cells": cell_changes.get(col, 0), }) return pd.DataFrame( rows, columns=["column", "before_dtype", "after_dtype", "changed_cells"] @@ -193,9 +221,10 @@ def to_dict(self) -> dict[str, Any]: "rows_after": self.rows_after, "cols_before": self.cols_before, "cols_after": self.cols_after, - "before_stats": self.before_stats, - "after_stats": self.after_stats, - "cell_changes": self.cell_changes, + # JSON object keys must be strings (tuple MultiIndex labels are not). + "before_stats": _str_keys(self.before_stats), + "after_stats": _str_keys(self.after_stats), + "cell_changes": _str_keys(self.cell_changes), "actions_by_step": self.actions_by_step, "narratives": self.narratives, "warnings": list(self.report.warnings), @@ -213,6 +242,8 @@ def explain_clean( """Run clean() and return a structured before/after explanation.""" cfg = merge_options(config, strategy=strategy, **options) df = to_pandas(df) # accept polars frames like the other public entry points + _require_unique_labels(df, "explain_clean") + _require_distinct_label_names(df, "explain_clean") before_stats = _column_stats(df) cleaned, report = run_pipeline(df, cfg) diff --git a/src/freshdata/render/renderers.py b/src/freshdata/render/renderers.py index a33fb6b..0bdb891 100644 --- a/src/freshdata/render/renderers.py +++ b/src/freshdata/render/renderers.py @@ -233,8 +233,9 @@ def render_explain(rep: Any) -> str: safe_rows: list[list[str]] = [] risky_rows: list[list[str]] = [] for col, changes in sorted(rep.cell_changes.items(), key=lambda kv: kv[1], reverse=True): - before = rep.before_stats.get(col, {}) - after = rep.after_stats.get(col, {}) + # Per-column dicts are keyed by the string form of the label. + before = rep.before_stats.get(str(col), {}) + after = rep.after_stats.get(str(col), {}) risky = bool(after.get("dtype") and before.get("dtype") and after["dtype"] != before["dtype"]) or changes > 0.5 * max( 1, rep.rows_before) diff --git a/tests/test_explain_column_labels.py b/tests/test_explain_column_labels.py new file mode 100644 index 0000000..5ebf4db --- /dev/null +++ b/tests/test_explain_column_labels.py @@ -0,0 +1,162 @@ +"""explain_clean / infer_roles with non-string and duplicate column labels. + +Regressions for #232 (parts 3 and 6) and #265 (part 3). +""" + +from __future__ import annotations + +import json +import warnings + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.explain import ExplainReport, _cell_changes + + +@pytest.fixture(autouse=True) +def _quiet(): + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + yield + + +def _int_label_frame() -> pd.DataFrame: + # Integer labels, as produced by ``pd.read_csv(header=None)``. + return pd.DataFrame({0: [" a", "b ", "c", "d"], 1: [1.0, 2.0, 3.0, 4.0]}) + + +def _multiindex_frame() -> pd.DataFrame: + return pd.DataFrame( + [[1, " a", None], [2, "b", 3.0]], + columns=pd.MultiIndex.from_tuples([("a", "x"), ("a", "y"), ("b", "z")]), + ) + + +def _mixed_label_frame() -> pd.DataFrame: + return pd.DataFrame({0: [1.0, None, 3.0, 4.0], "name": ["a", "b", None, "d"]}) + + +# -- #232 part 3: integer / tuple labels ------------------------------------- + + +def test_explain_integer_labels_key_every_mapping_by_string(): + rep = fd.explain_clean(_int_label_frame(), verbose=False) + for mapping in (rep.before_stats, rep.after_stats, rep.cell_changes): + assert set(mapping) == {"0", "1"} + assert rep.cell_changes["0"] == 2 + + +def test_explain_integer_labels_to_frame_reports_changed_cells(): + frame = fd.explain_clean(_int_label_frame(), verbose=False).to_frame() + row = frame.set_index("column").loc["0"] + assert row["changed_cells"] == 2 + assert row["before_dtype"] == "object" + assert row["after_dtype"] == "object" + + +def test_explain_integer_labels_html_shows_dtypes(): + html = fd.explain_clean(_int_label_frame(), verbose=False).to_html() + assert "