From 3609f584b50b364ca46d780388f0382604aaae83 Mon Sep 17 00:00:00 2001 From: Oscar Madera <80536682+oscarbol09@users.noreply.github.com> Date: Wed, 22 Jul 2026 13:34:48 -0500 Subject: [PATCH] fix(convert_salary_excel): pair count/index columns by category name, not adjacency (#219) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sequential scan assumed count/index pairs are always adjacent. Interleaved columns like Count_A, Count_B, Index_A, Index_B produced wrong pairings (Count_B ↔ Index_A), silently corrupting data. Now columns are grouped by type, then matched by the category name derived from stripping type words. Unmatched columns fall back to standalone value columns using the original header name. --- tests/test_convert_salary_excel.py | 25 +++++++++ tools/convert_salary_excel.py | 82 +++++++++++++++--------------- 2 files changed, 67 insertions(+), 40 deletions(-) diff --git a/tests/test_convert_salary_excel.py b/tests/test_convert_salary_excel.py index 2ce1ca2..0c03097 100644 --- a/tests/test_convert_salary_excel.py +++ b/tests/test_convert_salary_excel.py @@ -157,5 +157,30 @@ class DetectColumnTypeTests(unittest.TestCase): self.assertEqual(companies[0]["categories"]["salary_index"], {"index": 105.5}) + def test_parse_sheet_pairs_interleaved_count_index_columns_by_name(self): + ws = FakeWorksheet([ + ("Company", "Antal kvinder", "Antal mænd", "Kvinder indeks", "Mænd indeks"), + ("Example Corp", 15, 20, 95.0, 108.0), + ]) + + companies = parse_sheet(ws) + + categories = companies[0]["categories"] + self.assertEqual(categories["kvinder"], {"count": 15, "index": 95.0}) + self.assertEqual(categories["mænd"], {"count": 20, "index": 108.0}) + + def test_parse_sheet_non_adjacent_columns_no_cross_match(self): + ws = FakeWorksheet([ + ("Company", "Count_A", "Count_B", "Index_A", "Index_B"), + ("Example Corp", 10, 20, 100.0, 200.0), + ]) + + companies = parse_sheet(ws) + + categories = companies[0]["categories"] + self.assertEqual(categories["a"], {"count": 10, "index": 100.0}) + self.assertEqual(categories["b"], {"count": 20, "index": 200.0}) + + if __name__ == "__main__": unittest.main() diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index 30e28ab..8ff5573 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -132,53 +132,55 @@ def parse_sheet(ws, sheet_label=None): continue data_cols.append((i, h)) - # Try to detect paired count/index columns per category - # Heuristic: if columns come in pairs and alternate count/index, group them - categories = [] - i = 0 - while i < len(data_cols): - col_idx, col_header = data_cols[i] + # Group data columns by detected type and derive category names + count_cols = [] + index_cols = [] + untyped_cols = [] + + for col_idx, col_header in data_cols: col_type = detect_column_type(col_header) + if col_type == "count": + cat_name = strip_type_patterns(col_header, COUNT_PATTERNS) + count_cols.append((col_idx, col_header, cat_name)) + elif col_type == "index": + cat_name = strip_type_patterns(col_header, INDEX_PATTERNS) + index_cols.append((col_idx, col_header, cat_name)) + else: + untyped_cols.append((col_idx, col_header)) - if i + 1 < len(data_cols): - next_col_idx, next_col_header = data_cols[i + 1] - next_col_type = detect_column_type(next_col_header) + # Pair count/index columns by matching category name + categories = [] + used_counts = set() + used_indexes = set() - # If we have a count/index pair, group them - if col_type == "count" and next_col_type == "index": - # Use the header minus the count/index suffix as category name - cat_name = strip_type_patterns(col_header, COUNT_PATTERNS) - if not cat_name: - cat_name = f"category_{len(categories)+1}" - else: - cat_name = cat_name.replace(" ", "_").replace("-", "_") + for ci, (c_idx, c_header, c_cat) in enumerate(count_cols): + for ii, (i_idx, i_header, i_cat) in enumerate(index_cols): + if ii in used_indexes: + continue + if c_cat and i_cat and c_cat == i_cat: + cat_name = c_cat.replace(" ", "_").replace("-", "_") categories.append({ "name": cat_name, - "count_col": col_idx, - "index_col": next_col_idx, + "count_col": c_idx, + "index_col": i_idx, }) - i += 2 - continue - elif col_type == "index" and next_col_type == "count": - cat_name = strip_type_patterns(col_header, INDEX_PATTERNS) - if not cat_name: - cat_name = f"category_{len(categories)+1}" - else: - cat_name = cat_name.replace(" ", "_").replace("-", "_") - categories.append({ - "name": cat_name, - "index_col": col_idx, - "count_col": next_col_idx, - }) - i += 2 - continue + used_counts.add(ci) + used_indexes.add(ii) + break - # Single column - treat as a standalone value - categories.append({ - "name": col_header.lower().replace(" ", "_"), - "value_col": col_idx, - }) - i += 1 + # Remaining unmatched count columns become standalone (use original header) + for ci, (c_idx, c_header, _) in enumerate(count_cols): + if ci not in used_counts: + categories.append({"name": c_header.lower().replace(" ", "_"), "value_col": c_idx}) + + # Remaining unmatched index columns become standalone (use original header) + for ii, (i_idx, i_header, _) in enumerate(index_cols): + if ii not in used_indexes: + categories.append({"name": i_header.lower().replace(" ", "_"), "value_col": i_idx}) + + # Untyped columns become standalone + for col_idx, col_header in untyped_cols: + categories.append({"name": col_header.lower().replace(" ", "_"), "value_col": col_idx}) # Parse data rows companies = []