From 621ce5ab3952987776e6469c041dc898c5c9a463 Mon Sep 17 00:00:00 2001 From: Oscar Madera <80536682+oscarbol09@users.noreply.github.com> Date: Fri, 14 Aug 2026 03:51:06 -0500 Subject: [PATCH] fix(convert-salary-excel): reject ambiguous dot thousands separators (#326) --- CHANGELOG.md | 14 ++++++++++++++ tests/test_convert_salary_excel.py | 15 +++++++++++++++ tools/convert_salary_excel.py | 3 +++ 3 files changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b1774d..e9bc95d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,20 @@ prefer updating to a tagged release over pulling raw `master` (see files a release touched; `python3 tools/check_upstream_updates.py` lists them with per-file diff commands. +## [Unreleased] + +### Fixed + +- **`convert_salary_excel.py` no longer misreads whole-thousands cells from a Danish-locale + export** - a cell like `60.000` (thousands separator, no decimal comma) was handed to + `float()` and silently written as `60.0`, a 1000x-wrong salary in `salary_data.json` that + then rendered with a meaningless `vs baseline` percentage in `/apply`. The comma-side + mirror (`1,234`) was already guarded as ambiguous and skipped; the dot side had no guard, + and tests only pinned the both-separators form (`1.234,5`). `\d+\.\d{3}` is now rejected + the same way, so the shared never-guess policy applies to both separators and the rows in + between (e.g. `60.000,50`, `108,5`) keep parsing exactly as before. Pinned by + `tests/test_convert_salary_excel.py`. + ## [1.5.0] - 2026-08-12 ### Added diff --git a/tests/test_convert_salary_excel.py b/tests/test_convert_salary_excel.py index 1b954c2..e1ede29 100644 --- a/tests/test_convert_salary_excel.py +++ b/tests/test_convert_salary_excel.py @@ -230,6 +230,21 @@ class DetectColumnTypeTests(unittest.TestCase): self.assertEqual(companies[0]["categories"], {}) + def test_parse_sheet_skips_ambiguous_single_dot_thousands_string(self): + # "1.234" is the dot-side mirror of the comma guard above: in a + # decimal-dot locale it is 1.234, while a Danish export (whole + # thousands, no decimal comma, e.g. "60.000") means 1234/60000. + # float() used to write the 1000x-smaller value silently - the + # same never-guess policy must apply to both separators. + ws = FakeWorksheet([ + ("Company", "Salary Index"), + ("Example Corp", "1.234"), + ]) + + companies = parse_sheet(ws) + + self.assertEqual(companies[0]["categories"], {}) + 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"), diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index f334ef4..685fdfc 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -70,6 +70,9 @@ def parse_numeric_cell(value): if re.fullmatch(r"[+-]?\d+,\d{3}", text): raise ValueError("ambiguous comma separator") text = text.replace(",", ".") + elif "." in text: + if re.fullmatch(r"[+-]?\d+\.\d{3}", text): + raise ValueError("ambiguous dot separator") return float(text)