fix(explain): non-string column labels and duplicate-label errors in explain_clean and infer_roles - #381
Merged
Conversation
…abels 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)
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 was referenced Sep 15, 2026
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
Fixes non-string and duplicate column-label handling in
fd.explain_clean,fd.infer_rolesand the explain HTML renderer.ExplainReportkeyedbefore_stats/after_statsbystr(label)butcell_changesby the raw label, andrender_explainlooked stats up by the raw label. Integer-labelled columns showedchanged_cells = 0and blank HTML dtypes; MultiIndex tuple keys madeto_dict()/to_html()fail injson.dumps(_repr_html_()returnedNone). All per-column mappings are now keyed bystr(label),to_frame()/to_dict()stringify keys, and the renderer looks up bystr(label). Narratives now match actions (which recordstr(label)), so integer-labelled columns are no longer silently omitted.explain_cleanandinfer_rolessorted contexts by raw label and raisedTypeError; both now sort bystr(label).infer_roleskeeps the original label in itscolumnoutput (sodf[row["column"]]still works), passesstr(label)as the semantic-type name, and acceptssemantic_contextcolumn hints keyed by either form.ValueError("<func> requires unique column labels; duplicated: [...]"), matchingfill_missing, instead ofAttributeError/TypeError.explain_cleanalso raises aValueErrornaming labels whose string forms collide (e.g.1and"1") rather than silently merging them in its string-keyed report;infer_rolesaccepts such frames.Behaviour change:
ExplainReport.cell_changeskeys are now always strings (previously raw labels for non-string columns).Tests
New
tests/test_explain_column_labels.py: integer labels (keys,to_frame, HTML dtypes, narratives,_cell_changes), MultiIndex labels (to_dictJSON,_repr_html_,infer_roles),to_dict/to_framewith tuple keys, mixed int/str labels for both APIs, semantic hints keyed by string for integer labels, duplicate-labelValueErrorfor both APIs, and the1/"1"collision.Verification
ruff check .: passmypy src/freshdata: no issues (202 files)pytest -m "not online and not large": py3.12 4260 passed, 6 skipped; py3.9 4256 passed, 10 skipped (before rebasing onto current main; explain/API tests re-run after the rebase on both)Refs #232 (parts 3, 6)
Refs #265 (part 3)