Implement map wrangke using custom wrangle agent - #905
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new recipe-level map wrangle to support semantic/heuristic mapping of source column names onto a target schema, along with recipe tests and an end-to-end usage notebook.
Changes:
- Added
map(df, targets, ...)wrangle towrangles/recipe_wrangles/main.pyusing a similarity heuristic and greedy unique matching. - Added recipe-level test coverage for the new
mapwrangle intests/recipes/wrangles/test_main.py. - Added a user guidance notebook (
notebooks/map-wrangle-guide.ipynb) and an agent guide (.github/agents/wrangles.agent.md).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 15 comments.
| File | Description |
|---|---|
wrangles/recipe_wrangles/main.py |
Implements the new map wrangle and its schema/docstring. |
tests/recipes/wrangles/test_main.py |
Adds recipe execution tests for map behavior and parameters. |
notebooks/map-wrangle-guide.ipynb |
Provides a user-facing walkthrough and examples for map. |
.github/agents/wrangles.agent.md |
Adds an internal agent guide for creating wrangles/tests/docs. |
| items: | ||
| type: string |
There was a problem hiding this comment.
The schema block in this docstring appears invalid for JSON schema/YAML-style docs: items is at the same indentation level as targets, so it won’t be associated with the targets array. items should be nested under properties.targets (like other array properties in this file), otherwise schema generation / editor hints may break.
| items: | |
| type: string | |
| items: | |
| type: string |
There was a problem hiding this comment.
I don't get how items would fit under targets. Perhaps it shouldn't be there at all?
| def _normalize(name: str) -> str: | ||
| s = name if case_sensitive else name.lower() | ||
| s = _re.sub(r'[_\-]+', ' ', s) | ||
| s = _re.sub(r'[^a-z0-9 ]+', ' ', s) |
There was a problem hiding this comment.
When case_sensitive=True, _normalize() does not lowercase but still applies [^a-z0-9 ]+, which strips A–Z characters. This makes common column names like "SKU" normalize to an empty string and can break similarity scoring. Consider allowing uppercase in the regex (e.g., include A-Z) or applying case handling in a way that preserves letters while still respecting case sensitivity.
| s = _re.sub(r'[^a-z0-9 ]+', ' ', s) | |
| allowed_chars_pattern = r'[^A-Za-z0-9 ]+' if case_sensitive else r'[^a-z0-9 ]+' | |
| s = _re.sub(allowed_chars_pattern, ' ', s) |
| # Build all candidate pairs with scores | ||
| pairs = [] | ||
| for col in candidate_cols: | ||
| for tgt in remaining_targets: | ||
| # Avoid identity pairs to prevent trivial self-mapping | ||
| if str(col) == str(tgt): | ||
| continue | ||
| pairs.append((col, tgt, _similar(col, tgt))) | ||
|
|
||
| # Sort by descending similarity | ||
| pairs.sort(key=lambda x: x[2], reverse=True) | ||
|
|
||
| used_cols = set() | ||
| used_targets = set() | ||
| for col, tgt, score in pairs: | ||
| if col in used_cols or tgt in used_targets: | ||
| continue | ||
| if score >= threshold: | ||
| rename_dict[col] = tgt | ||
| used_cols.add(col) | ||
| used_targets.add(tgt) | ||
|
|
||
| # Handle unmapped columns | ||
| if drop_unmapped: | ||
| drop_cols = [c for c in candidate_cols if c not in rename_dict] | ||
| if drop_cols: | ||
| df = df.drop(columns=drop_cols) | ||
|
|
||
| # Drop any existing columns that would be overwritten by the rename | ||
| if rename_dict: | ||
| outputs = set(rename_dict.values()) | ||
| sources = set(rename_dict.keys()) | ||
| drop_conflicts = [c for c in outputs if c in df.columns and c not in sources] | ||
| if drop_conflicts: | ||
| df = df.drop(columns=drop_conflicts) | ||
| return df.rename(columns=rename_dict) |
There was a problem hiding this comment.
Current conflict handling can silently drop an existing target column and replace it with a renamed source (data loss). Example: if df already has a price column and another column maps to target price, drop_conflicts will drop the existing price because identity pairs are skipped and the target isn’t treated as already satisfied. Consider marking exact target matches as already-used targets (or giving precedence to existing exact-match columns) so standardized columns aren’t overwritten unintentionally.
| wrangles: | ||
| - map: | ||
| targets: | ||
| - name | ||
| - price | ||
| - quantity |
There was a problem hiding this comment.
These embedded YAML recipes have incorrect indentation for the targets list: the - name/- price entries are aligned with targets: rather than indented under it. This is likely to fail YAML parsing in wrangles.recipe.run(). Please indent list items under targets: (and similarly for other lists in this test class).
| - map: | ||
| input: | ||
| - Product Name | ||
| - Qty | ||
| targets: | ||
| - name | ||
| - quantity |
There was a problem hiding this comment.
YAML indentation issue: list items under input: and targets: need to be indented beneath their keys. As written, the - Product Name / - Qty and target entries are aligned with the keys, which likely breaks YAML parsing.
| wrangles: | ||
| - map: | ||
| targets: | ||
| - sku | ||
| - name | ||
| case_sensitive: false | ||
| """ |
There was a problem hiding this comment.
YAML indentation issue: list items under targets: should be indented beneath targets: in this recipe string.
| wrangles: | ||
| - map: | ||
| targets: | ||
| - sku | ||
| - name | ||
| case_sensitive: true | ||
| threshold: 0.9 |
There was a problem hiding this comment.
YAML indentation issue: list items under targets: should be indented beneath targets: in this recipe string.
| "})\n", | ||
| "\n", | ||
| "# Subset mapping example\n", | ||
| "df_subset = df_basic.copy()\n", |
There was a problem hiding this comment.
The subset example references a Notes column in the assertion, but df_subset is created as df_basic.copy() and doesn’t include Notes. This makes the example confusing and the assertion non-deterministic (... or ...). Consider adding a Notes column to df_subset (to demonstrate the “unmapped columns are retained” behavior) and asserting a single expected column list.
| "df_subset = df_basic.copy()\n", | |
| "df_subset = df_basic.copy()\n", | |
| "df_subset[\"Notes\"] = [\"Priority item\"]\n", |
| You are a **Wrangle Creator** specializing in **data transformation functions for the WranglesPY repository**. You focus on **pure, typed, vectorized pandas operations** and **comprehensive test coverage**. You are the expert in creating production-ready wrangles from YAML specifications. | ||
|
|
||
| ## ⚡ Capabilities | ||
| - **Quality Enforcement:** Ensure all code passes flake8, mypy, and achieves ≥85% test coverage. |
There was a problem hiding this comment.
This agent guide states that PRs must pass flake8 and mypy, but there doesn’t appear to be any flake8/mypy configuration or workflow enforcement in this repo. If those tools aren’t actually part of the project’s quality gates, consider removing/softening those requirements or documenting how to run them so contributors aren’t misled.
| - **Quality Enforcement:** Ensure all code passes flake8, mypy, and achieves ≥85% test coverage. | |
| - **Quality Enforcement:** Follow existing code patterns, add comprehensive pytest coverage, and document practical verification steps for contributors. |
| wrangles: | ||
| - map: | ||
| targets: | ||
| - a | ||
| threshold: -0.1 | ||
| """ |
There was a problem hiding this comment.
YAML indentation issue in this error-case recipe: under targets: the - a item needs to be indented beneath the key; as written it’s aligned with targets: and is likely invalid YAML.
thomasstvr
left a comment
There was a problem hiding this comment.
@mborodii-prog copilot made some good suggestions. I did not go through all 15 of them, but I'm sure some are better than others while some might be meaningless. Please go through copilot's comments and make changes as needed.
One thing that jumps out to me is that all targets should be created, regardless of meeting the threshold in order to match the functionality of a map wrangle. So, if nothing meets the threshold or there are more targets than existing columns, the wrangle should simply create those as empty columns.
This also seems like a good opportunity to use recipes within our code. I'm not sure how exactly, but it just seems like a recipe could be used for renaming matches and creating columns.
|
Queue triage (2026-07-27)
GitHub is the status record; update this PR rather than the external spreadsheet. |
Add map wrangle with tests and notebook guide
Summary
Introduces a new map wrangle for semantic column name mapping in recipes.
Adds comprehensive recipe-level tests and a user-facing guidance notebook.
Handles edge cases (identity mappings, conflicting targets) and supports case sensitivity.
Motivation
Simplifies aligning messy or varied source column names to a standard schema.
Reduces manual renaming and improves recipe portability across datasets.
Changes
New wrangle: main.py (map).
Tests: test_main.py (TestMap).
Docs: map-wrangle-guide.ipynb.
Implementation
Greedy unique matching between input columns and targets.
Skips identity self-mappings to avoid reserving targets unnecessarily.
Drops existing conflicting target columns before renaming to prevent duplicates.
Similarity blends SequenceMatcher ratio with target-token coverage; no stopwords.
Parameters:
targets: list[str], required.
input: optional list; supports “?” suffix for optional input.
threshold: float [0–1], defaults 0.6.
drop_unmapped: bool.
case_sensitive: bool, default False.
Tests
Scenarios: basic mapping, subset input, threshold filtering, conflict resolution, overwriting existing targets, invalid parameters, case-insensitive and case-sensitive behavior.
Documentation
Notebook demonstrates usage end-to-end with assertions and includes sections for error handling and best-practice tips.
Usage Example
Risks
Heuristic mapping may need tuning (threshold, case_sensitive) for certain datasets.
Large schemas incur more comparisons; mitigate by providing an input subset.
Checklist