fix: report revert, outlier NA mask, pyarrow error message

- CleanReport.revert() wrote reverted values into the caller's frame: the
  shallow copy's object columns still share memory with the input, and the
  .loc assignment landed in it. Revert now works on its own column copy.
- detect_outliers() returned a mask with <NA> on nullable numeric columns and
  remove_outliers() then dropped rows that were only missing. Missing cells are
  now "not an outlier" and the mask is plain bool.
- remove_outliers()/resolve_duplicates() with inplace=True dropped by label, so
  a duplicated index also removed unflagged rows; they now raise a clear
  ValueError instead of over-dropping (the copy path is unchanged).
- require_pyarrow() hard-coded "Reading Parquet metadata" even for
  output_format="arrow"; it now names the feature. from_parquet_path() also
  relied on pyarrow.parquet having been imported elsewhere and raised
  AttributeError in a fresh process; it imports the submodule itself.

Closes #207
Closes #211
Closes #215

Author: kevincostner17

src/freshdata/execution/__init__.py

@@ -124,7 +124,7 @@ def _convert_output(frame: Any, output_format: str) -> Any:
     if output_format == "arrow":
         from ._lazy import require_pyarrow
 
-        require_pyarrow()
+        require_pyarrow("Arrow output (output_format='arrow')")
         import pyarrow as pa
 
         if isinstance(frame, pa.Table):

src/freshdata/execution/_lazy.py

@@ -35,13 +35,17 @@ def require_duckdb() -> Any:
     return duckdb
 
 
-def require_pyarrow() -> Any:
-    """Return the imported :mod:`pyarrow` module or raise a helpful error."""
+def require_pyarrow(purpose: str = "This feature") -> Any:
+    """Return the imported :mod:`pyarrow` module or raise a helpful error.
+
+    *purpose* names what needs pyarrow (e.g. ``"Arrow output"``) so the error
+    points at the feature the caller actually used.
+    """
     try:
         import pyarrow  # noqa: F401
-    except ImportError as exc:  # pragma: no cover - exercised via message
+    except ImportError as exc:
         raise ImportError(
-            "Reading Parquet metadata requires pyarrow. "
+            f"{purpose} requires pyarrow. "
             "Install it with: pip install 'freshdata-cleaner[pyarrow]'"
         ) from exc
     return pyarrow

src/freshdata/execution/_metadata.py

@@ -174,7 +174,12 @@ def from_parquet_path(path: str) -> list[ColumnMetadata]:
         The footer gives the row count for free (no data scan); DuckDB streams the
         file for null/value statistics without loading it into Python.
         """
-        n_rows = require_pyarrow().parquet.read_metadata(path).num_rows
+        require_pyarrow("Reading Parquet metadata")
+        # ``pyarrow.parquet`` is a submodule: importing ``pyarrow`` alone does
+        # not expose it as an attribute.
+        import pyarrow.parquet as pq
+
+        n_rows = pq.read_metadata(path).num_rows
         duckdb = require_duckdb()
         escaped = path.replace("'", "''")
         conn = duckdb.connect()

src/freshdata/report.py

@@ -446,7 +446,9 @@ def revert(
         for column, column_entries in touched.items():
             if column not in out.columns:
                 continue
-            series = out[column]
+            # Own copy: an object column of a shallow copy still shares memory
+            # with the caller's frame, and ``.loc`` below would write into it.
+            series = out[column].copy()
             if series.dtype != object:
                 series = series.astype(object)
             for entry in column_entries:

src/freshdata/simple.py

@@ -110,10 +110,29 @@ def _outlier_mask(
             lo, hi = mean - factor * std, mean + factor * std
         if pd.isna(lo) or pd.isna(hi) or lo == hi:
             continue  # all-null / constant column -- nothing to flag
-        mask |= (s < lo) | (s > hi)
+        # Nullable dtypes compare to <NA> on missing cells; missing is not an outlier.
+        mask |= ((s < lo) | (s > hi)).fillna(False).astype(bool)
     return mask
 
 
+def _drop_rows_inplace(df: pd.DataFrame, mask: pd.Series, func: str) -> None:
+    """Drop the rows flagged by *mask* from *df* in place.
+
+    ``DataFrame.drop`` works by label, so on a non-unique index it would also
+    remove unflagged rows sharing a label with a flagged one. Refuse instead of
+    silently over-dropping.
+    """
+    if not mask.any():
+        return
+    if not df.index.is_unique:
+        raise ValueError(
+            f"{func}(inplace=True) requires a unique index; dropping by label would "
+            "also remove unflagged rows that share a label. Use inplace=False or "
+            "reset the index first."
+        )
+    df.drop(index=df.index[mask.to_numpy()], inplace=True)
+
+
 def fill_missing(
     df: pd.DataFrame,
     columns: str | Sequence[str] | None = None,
@@ -216,7 +235,7 @@ def remove_outliers(
     cols = _resolve_columns(df, columns, numeric_only=True)
     mask = _outlier_mask(df, cols, method, threshold)
     if inplace:
-        df.drop(index=df.index[mask], inplace=True)
+        _drop_rows_inplace(df, mask, "remove_outliers")
         result = df
     else:
         result = df.loc[~mask]
@@ -246,7 +265,7 @@ def resolve_duplicates(
     keep: str | bool = False if method == "drop" else method
     drop_mask = df.duplicated(subset=subset, keep=keep)
     if inplace:
-        df.drop(index=df.index[drop_mask], inplace=True)
+        _drop_rows_inplace(df, drop_mask, "resolve_duplicates")
         result = df
     else:
         result = df.loc[~drop_mask]

tests/test_execution/test_pyarrow_requirement.py

@@ -0,0 +1,57 @@
+"""pyarrow requirement errors name the feature that needs it (#215)."""
+
+from __future__ import annotations
+
+import subprocess
+import sys
+
+import pandas as pd
+import pytest
+
+import freshdata as fd
+from freshdata.execution._lazy import require_pyarrow
+from freshdata.execution._metadata import MetadataScanner
+
+
+@pytest.fixture
+def no_pyarrow(monkeypatch):
+    # A ``None`` entry makes ``import pyarrow`` (and its submodules) raise ImportError.
+    monkeypatch.setitem(sys.modules, "pyarrow", None)
+
+
+def test_arrow_output_error_names_arrow_output(no_pyarrow):
+    with pytest.raises(ImportError) as exc:
+        fd.clean(pd.DataFrame({"a": [1.0, None]}), output_format="arrow", verbose=False)
+    message = str(exc.value)
+    assert "Arrow output" in message
+    assert "Parquet" not in message
+    assert "freshdata-cleaner[pyarrow]" in message
+
+
+def test_parquet_metadata_error_names_parquet(no_pyarrow):
+    with pytest.raises(ImportError, match="Reading Parquet metadata requires pyarrow"):
+        MetadataScanner.from_parquet_path("missing.parquet")
+
+
+def test_default_purpose(no_pyarrow):
+    with pytest.raises(ImportError, match="This feature requires pyarrow"):
+        require_pyarrow()
+
+
+def test_parquet_metadata_in_fresh_process(tmp_path):
+    # ``pyarrow.parquet`` is not an attribute of a bare ``import pyarrow``; the
+    # scanner must import the submodule itself rather than rely on a caller.
+    pytest.importorskip("pyarrow")
+    pytest.importorskip("duckdb")
+    path = tmp_path / "x.parquet"
+    pd.DataFrame({"a": [1.0, None, 3.0]}).to_parquet(path)
+    code = (
+        "from freshdata.execution._metadata import MetadataScanner as M;"
+        f"m = M.from_parquet_path({str(path)!r})[0];"
+        "print(m.row_count, m.non_null_count)"
+    )
+    result = subprocess.run(
+        [sys.executable, "-c", code], capture_output=True, text=True, check=False
+    )
+    assert result.returncode == 0, result.stderr
+    assert result.stdout.split() == ["3", "2"]

tests/test_report.py

@@ -241,6 +241,33 @@ def test_revert_restores_values_and_skips_missing_columns():
     assert "missing_column" not in restored.columns
 
 
+def test_revert_does_not_mutate_input_object_column():
+    report = fd.CleanReport()
+    report.undo_log = {
+        "entries": [{"action_id": "a1", "column": "email", "index": [0], "value": " A@X.COM"}],
+        "column_dtypes": {"email": "object"},
+    }
+    df = pd.DataFrame({"email": ["a@x.com", "b@y.org"]})
+    snapshot = df.copy(deep=True)
+
+    restored = report.revert(df)
+
+    pd.testing.assert_frame_equal(df, snapshot)
+    assert restored["email"].tolist() == [" A@X.COM", "b@y.org"]
+
+
+def test_revert_after_apply_plan_leaves_cleaned_frame_untouched():
+    df = pd.DataFrame({"email": [" A@X.COM", "b@y.org", "C@Z.COM "], "n": [1, 2, 3]})
+    plan = fd.suggest_plan(df, context="email must be a valid email.")
+    cleaned, report = fd.apply_plan(df, plan, keep_undo=True)
+    snapshot = cleaned.copy(deep=True)
+
+    restored = report.revert(cleaned)
+
+    pd.testing.assert_frame_equal(cleaned, snapshot)
+    assert restored["email"].tolist() == df["email"].tolist()
+
+
 def test_action_str_format():
     action = fd.Action(step="impute", column="age", description="filled 2", count=2)
     assert str(action) == "[impute] 'age': filled 2"

tests/test_simple.py

@@ -156,6 +156,37 @@ def test_remove_outliers_inplace():
     assert len(df) == 5
 
 
+def _nullable_with_gap():
+    return pd.DataFrame(
+        {"x": pd.array([10, 11, 12, 13, 12, 11, 10, 1000, None], dtype="Int64")}
+    )
+
+
+def test_detect_outliers_nullable_mask_is_plain_bool():
+    mask = fd.detect_outliers(_nullable_with_gap())
+    assert mask.dtype == bool
+    assert mask.tolist()[-2:] == [True, False]  # missing is not an outlier
+
+
+@pytest.mark.parametrize("inplace", [False, True])
+def test_remove_outliers_keeps_missing_rows_in_nullable_columns(inplace):
+    df = _nullable_with_gap()
+    expected = fd.remove_outliers(df.astype("float64")).index.tolist()
+    out = fd.remove_outliers(df, inplace=inplace)
+    assert out.index.tolist() == expected == [0, 1, 2, 3, 4, 5, 6, 8]
+
+
+def test_remove_outliers_inplace_rejects_duplicate_index():
+    df = pd.DataFrame({"x": [1, 2, 3, 4, 5, 1000]}, index=[0, 1, 2, 3, 4, 0])
+    with pytest.raises(ValueError, match="unique index"):
+        fd.remove_outliers(df, inplace=True)
+    assert len(df) == 6  # untouched
+    assert fd.remove_outliers(df).index.tolist() == [0, 1, 2, 3, 4]
+    # nothing flagged -> nothing to drop, so a duplicate index is fine
+    calm = pd.DataFrame({"x": [1, 2, 3]}, index=[0, 0, 1])
+    assert fd.remove_outliers(calm, inplace=True) is calm
+
+
 def test_remove_outliers_bad_method_raises():
     with pytest.raises(ValueError, match="method must be one of"):
         fd.remove_outliers(pd.DataFrame({"x": [1]}), method="nope")