mirror of
https://github.com/MadsLorentzen/ai-job-search.git
synced 2026-09-17 08:36:25 +00:00
fix(salary-tools): decide number locale by last separator, pair compound headers
Review findings F7 and F8 (2026-08-19):
- parse_numeric_cell's both-separators branch always assumed European
locale, silently turning a US "1,234.56" into 1.23456 - a 1000x
corruption written to salary_data.json with no warning. The separator
that appears last is now treated as the decimal separator, which also
makes multi-group values ("1,234,567.89") parse instead of raising a
raw float error. Single-separator ambiguity guards are unchanged.
- strip_type_patterns stripped only whole tokens, so the compound header
"Lønindeks alle" kept its type word and could never pair with "Antal
alle" - failing exactly for the compound-word locale COMPOUND_PATTERNS
exists to support. It now also strips compound patterns as substrings,
mirroring header_matches.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
7aba0b4a9d
commit
1c19f6c45f
@@ -56,6 +56,16 @@ per-file diff commands.
|
|||||||
|
|
||||||
### Fixed
|
### 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
|
- **`jobdanmark-search` extracts the city when a comma follows the postcode** - the
|
||||||
`location` regex required whitespace after the 4-digit postcode, but live
|
`location` regex required whitespace after the 4-digit postcode, but live
|
||||||
`companyAddress` values frequently read `"2670, Greve"`; those results emitted
|
`companyAddress` values frequently read `"2670, Greve"`; those results emitted
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ from tools.convert_salary_excel import (
|
|||||||
INDEX_PATTERNS,
|
INDEX_PATTERNS,
|
||||||
detect_column_type,
|
detect_column_type,
|
||||||
header_matches,
|
header_matches,
|
||||||
|
parse_numeric_cell,
|
||||||
parse_sheet,
|
parse_sheet,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -288,3 +289,54 @@ class DetectColumnTypeTests(unittest.TestCase):
|
|||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
unittest.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},
|
||||||
|
)
|
||||||
|
|||||||
@@ -65,7 +65,13 @@ def parse_numeric_cell(value):
|
|||||||
if not text:
|
if not text:
|
||||||
raise ValueError("not numeric")
|
raise ValueError("not numeric")
|
||||||
if "," in text and "." in text:
|
if "," in text and "." in text:
|
||||||
|
# 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(",", ".")
|
text = text.replace(".", "").replace(",", ".")
|
||||||
|
else:
|
||||||
|
text = text.replace(",", "")
|
||||||
elif "," in text:
|
elif "," in text:
|
||||||
if re.fullmatch(r"[+-]?\d+,\d{3}", text):
|
if re.fullmatch(r"[+-]?\d+,\d{3}", text):
|
||||||
raise ValueError("ambiguous comma separator")
|
raise ValueError("ambiguous comma separator")
|
||||||
@@ -95,10 +101,18 @@ def header_matches(header, patterns):
|
|||||||
|
|
||||||
|
|
||||||
def strip_type_patterns(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()
|
name = header.lower()
|
||||||
for p in patterns:
|
for p in patterns:
|
||||||
name = re.sub(rf"(?<![a-zæøåöäü0-9]){re.escape(p)}(?![a-zæøåöäü0-9])", "", name)
|
name = re.sub(rf"(?<![a-zæøåöäü0-9]){re.escape(p)}(?![a-zæøåöäü0-9])", "", name)
|
||||||
|
if p in COMPOUND_PATTERNS:
|
||||||
|
name = name.replace(p, "")
|
||||||
return name.strip(" _-")
|
return name.strip(" _-")
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user