fix(guard,explain): survive duplicate index labels and unhashable cells - #452
Merged
Merged
Conversation
The protected-column guard compared its snapshot with before.loc[after.index]. Repeated index labels make that reindex multiply rows instead of selecting them, so once any row was dropped the comparison could never match and fd.clean raised ProtectedColumnError on a column it had not touched - telling the user to report an executor bug. Context policies and mutable=False were therefore unusable on any frame with a non-unique index. When either index has repeated labels the guard now checks positionally: row-level steps only drop rows, so every surviving value must still appear, in order, in the original column. Rewritten cells, reordered rows and gained rows are still reported, and two missing values still count as equal. explain_clean counted distinct values with nunique(), which hashes every value, so a list or dict cell raised TypeError although fd.clean and fd.profile accept the same frame. Unhashable cells are now counted by their text form, as profile already does. Closes #449 Closes #450
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 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. Comment |
FreshData benchmark report —
|
| fixture | n_rows | n_cols | p50 s | p95 s | peak MB | repair % | false-repair % | preserve % | trust | monotonic | export % |
|---|
Authored-code reduction (Metric 6)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two crashes on frames that
fd.cleanandfd.profileotherwise accept, both found by fuzzing.ProtectedColumnErroron a frame with duplicate index labels, on a column it did not touchfd.explain_cleanraisesTypeError: unhashable type: 'list'Root cause
_series_identicalaligned its snapshot withbefore.loc[after.index]. With repeated labels that reindex multiplies rows instead of selecting them, so as soon as a row-level step dropped any row (heredrop_empty_rows), the lengths could never line up and the guard reported a violation. The column was in fact returned unchanged, and the error told the user to report a freshdata executor bug. Net effect: context policies andmutable=Falsewere unusable on any frame with a non-unique index._column_statscounts distinct values withnunique(), which hashes every value, so one list or dict cell aborted the whole report.profile.pyalready guards the same pattern.Behaviour change
explain_clean: unhashable cells are counted by their text form, so[1],[1],{"k": 2}counts as 2 distinct values. Hashable columns are untouched.Default-output changes
None for frames that already worked.
ProtectedColumnError. The guard action in the report is recorded as verified rather than violated.explain_cleanon a frame with list or dict cells now returns a report with anuniquecount where it used to raise.Tests
tests/test_guard_protected.py:drop_empty_rows) cleans and matches the unprotected result_series_identicalon a duplicate index still reports a rewritten cell, reordered rows and gained rowstests/test_explain.py:explain_cleanon a frame with list and dict cells returns a report, counts 2 distinct payloads and still counts the null.Verification
ruff checkandmypy src/freshdata: clean.main.not online and not largelanes: py3.12 6507 passed, 17 skipped; py3.9 6478 passed, 21 skipped.