diff --git a/CHANGELOG.md b/CHANGELOG.md index 1887902..c067eb1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,16 @@ per-file diff commands. ### Fixed +- **`convert_salary_excel.py` no longer corrupts US/UK-formatted numbers 1000x** - the + both-separators branch always assumed European locale, so a `"1,234.56"` cell was + silently converted to `1.23456` and written into `salary_data.json`. The rule is now + "the separator that appears last is the decimal separator", which also makes + multi-group values (`"1,234,567.89"`, `"1.234.567,89"`) parse instead of raising. And + `strip_type_patterns` now strips `COMPOUND_PATTERNS` words as substrings, mirroring + `header_matches`, so a Danish compound header pair ("Antal alle" / "Lønindeks alle") + pairs into one category instead of two unpaired standalones - the exact locale the + compound support was added for. Pinned by six new cases in + `tests/test_convert_salary_excel.py`. - **`jobdanmark-search` extracts the city when a comma follows the postcode** - the `location` regex required whitespace after the 4-digit postcode, but live `companyAddress` values frequently read `"2670, Greve"`; those results emitted diff --git a/tests/test_convert_salary_excel.py b/tests/test_convert_salary_excel.py index e1ede29..665825e 100644 --- a/tests/test_convert_salary_excel.py +++ b/tests/test_convert_salary_excel.py @@ -5,6 +5,7 @@ from tools.convert_salary_excel import ( INDEX_PATTERNS, detect_column_type, header_matches, + parse_numeric_cell, parse_sheet, ) @@ -288,3 +289,54 @@ class DetectColumnTypeTests(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +class ParseNumericCellLocaleTests(unittest.TestCase): + # The separator that appears LAST is the decimal separator. Assuming + # European ("." thousands, "," decimal) for every both-separator string + # turned a US "1,234.56" into 1.23456 - a silent 1000x corruption that + # flowed into salary_data.json and negotiation advice. + + def test_us_thousands_and_decimal_string(self): + self.assertEqual(parse_numeric_cell("1,234.56"), 1234.56) + + def test_us_multiple_thousands_groups(self): + self.assertEqual(parse_numeric_cell("1,234,567.89"), 1234567.89) + + def test_european_thousands_and_decimal_string(self): + self.assertEqual(parse_numeric_cell("1.234,56"), 1234.56) + + def test_european_multiple_thousands_groups(self): + self.assertEqual(parse_numeric_cell("1.234.567,89"), 1234567.89) + + +class CompoundCategoryPairingTests(unittest.TestCase): + def test_parse_sheet_pairs_danish_compound_index_with_count(self): + # "Lønindeks alle" is *detected* as an index column via + # COMPOUND_PATTERNS, but the derived category name must also lose the + # compound word or it can never pair with "Antal alle" ("alle" vs + # "lønindeks alle") - exactly the locale the compound support exists for. + ws = FakeWorksheet([ + ("Firma", "Antal alle", "Lønindeks alle"), + ("Example Corp", 12, 118.0), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["alle"], + {"count": 12, "index": 118.0}, + ) + + def test_parse_sheet_sheet_level_us_locale_value(self): + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1,234.56"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual( + companies[0]["categories"]["salary_index"], + {"index": 1234.56}, + ) diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index 685fdfc..2e8b6bb 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -65,7 +65,13 @@ def parse_numeric_cell(value): if not text: raise ValueError("not numeric") if "," in text and "." in text: - text = text.replace(".", "").replace(",", ".") + # The separator that appears last is the decimal separator: European + # "1.234,56" and US "1,234.56" are both unambiguous here, unlike the + # single-separator cases below. + if text.rfind(",") > text.rfind("."): + text = text.replace(".", "").replace(",", ".") + else: + text = text.replace(",", "") elif "," in text: if re.fullmatch(r"[+-]?\d+,\d{3}", text): raise ValueError("ambiguous comma separator") @@ -95,10 +101,18 @@ def header_matches(header, patterns): def strip_type_patterns(header, patterns): - """Remove count/index words from a header to derive a category name.""" + """Remove count/index words from a header to derive a category name. + + Mirrors ``header_matches``: patterns strip as whole tokens, and any + pattern also listed in ``COMPOUND_PATTERNS`` additionally strips as a + substring - otherwise a compound header like "Lønindeks alle" keeps the + type word in its category name and can never pair with "Antal alle". + """ name = header.lower() for p in patterns: name = re.sub(rf"(?