fix(salary): skip non-numeric and identifier columns in Excel conversion (#152)

parse_sheet treated every column that was not company/city as a salary category, with no check that the column actually held numeric salary data. This turned free-text columns (e.g. Notes) into bogus string categories and numeric identifier columns (e.g. Id) into mistaken salary indexes.

- Drop identifier headers (ID_PATTERNS = {id, personnummer}) at classification time.

- Skip non-numeric standalone values and fully-null count/index pairs at row-processing time.

- Adds regression tests (skips_free_text_column, skips_numeric_identifier_column, keeps_numeric_salary_column) that fail on master and pass after the fix.
This commit is contained in:
Alaa-Taieb
2026-07-14 20:11:11 +02:00
committed by GitHub
parent 4128ca0318
commit 1417e3cbdf
2 changed files with 50 additions and 2 deletions
+36
View File
@@ -104,6 +104,42 @@ class DetectColumnTypeTests(unittest.TestCase):
companies[0]["categories"]["salary"], {"index": 105.5} companies[0]["categories"]["salary"], {"index": 105.5}
) )
def test_skips_free_text_column(self):
# A free-text "Notes" column must not become a bogus salary category.
ws = FakeWorksheet([
("Company", "Salary Index", "Notes"),
("Example Corp", 105.5, "good"),
])
companies = parse_sheet(ws)
self.assertIn("salary_index", companies[0]["categories"])
self.assertNotIn("notes", companies[0]["categories"])
def test_skips_numeric_identifier_column(self):
# A numeric "Id" column (employee id) must not be treated as a salary index.
ws = FakeWorksheet([
("Company", "Salary Index", "Id"),
("Example Corp", 105.5, 7),
])
companies = parse_sheet(ws)
self.assertIn("salary_index", companies[0]["categories"])
self.assertNotIn("id", companies[0]["categories"])
def test_keeps_numeric_salary_column(self):
# A genuine numeric salary column still produces a salary category.
ws = FakeWorksheet([
("Company", "Salary Index"),
("Example Corp", 105.5),
])
companies = parse_sheet(ws)
self.assertIn("salary_index", companies[0]["categories"])
self.assertEqual(companies[0]["categories"]["salary_index"], {"index": 105.5})
if __name__ == "__main__": if __name__ == "__main__":
unittest.main() unittest.main()
+14 -2
View File
@@ -48,6 +48,10 @@ INDEX_PATTERNS = {"indeks", "index", "idx", "salary", "løn", "median", "average
# Ships populated for this repo's Danish demonstration data; a fork targeting # Ships populated for this repo's Danish demonstration data; a fork targeting
# another locale edits this constant. # another locale edits this constant.
COMPOUND_PATTERNS = {"antal", "indeks", "løn", "gennemsnit", "medarbejdere"} COMPOUND_PATTERNS = {"antal", "indeks", "løn", "gennemsnit", "medarbejdere"}
# Identifier columns (employee id, Danish "personnummer", etc.) are never salary
# data. They are dropped at classification so they are not mistaken for a salary
# category. Matched as whole tokens only, like other pattern sets.
ID_PATTERNS = {"id", "personnummer"}
def header_matches(header, patterns): def header_matches(header, patterns):
@@ -120,11 +124,13 @@ def parse_sheet(ws, sheet_label=None):
print(f"Warning: Could not find company column in sheet '{ws.title}'.", file=sys.stderr) print(f"Warning: Could not find company column in sheet '{ws.title}'.", file=sys.stderr)
return [] return []
# Identify data columns (everything that's not company/city) # Identify data columns (everything that's not company/city or an identifier)
data_cols = [] data_cols = []
for i, h in enumerate(headers): for i, h in enumerate(headers):
if i == company_col or i == city_col or not h: if i == company_col or i == city_col or not h:
continue continue
if header_matches(h, ID_PATTERNS):
continue
data_cols.append((i, h)) data_cols.append((i, h))
# Try to detect paired count/index columns per category # Try to detect paired count/index columns per category
@@ -205,6 +211,10 @@ def parse_sheet(ws, sheet_label=None):
index_val = float(row[cat["index_col"]]) index_val = float(row[cat["index_col"]])
except (ValueError, TypeError): except (ValueError, TypeError):
pass pass
# A count/index pair that is entirely empty for this row carries
# no salary information, so skip it rather than emit nulls.
if count_val is None and index_val is None:
continue
entry["categories"][cat_name] = {"count": count_val, "index": index_val} entry["categories"][cat_name] = {"count": count_val, "index": index_val}
elif "value_col" in cat: elif "value_col" in cat:
if cat["value_col"] < len(row) and row[cat["value_col"]] is not None: if cat["value_col"] < len(row) and row[cat["value_col"]] is not None:
@@ -212,7 +222,9 @@ def parse_sheet(ws, sheet_label=None):
try: try:
val = float(val) val = float(val)
except (ValueError, TypeError): except (ValueError, TypeError):
val = str(val) # Non-numeric standalone value (e.g. a free-text "Notes"
# column) is not salary data; skip it for this row.
continue
entry["categories"][cat_name] = {"index": val} entry["categories"][cat_name] = {"index": val}
companies.append(entry) companies.append(entry)