diff --git a/CHANGELOG.md b/CHANGELOG.md index 803a916f..2d3feded 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -308,6 +308,11 @@ per-file diff commands. now a date pair; `1500–2000` and `2000–5000` stay quantities. Two new `test_verify_pdf.py` cases; the date-pair one fails on master. +- **Excel logical cells no longer become salary numbers** (#664, `tools/convert_salary_excel.py`) - + `TRUE` and `FALSE` cells are skipped as non-numeric instead of turning into counts or + salary indices of 1 and 0. Numeric 0 and 1 remain valid, and a count/index pair retains + its numeric half when the other cell is logical. + ### Security - **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a diff --git a/tests/test_convert_salary_excel.py b/tests/test_convert_salary_excel.py index 5518e541..8096a4f3 100644 --- a/tests/test_convert_salary_excel.py +++ b/tests/test_convert_salary_excel.py @@ -1,8 +1,18 @@ import io +import json +from pathlib import Path +import subprocess +import sys +import tempfile import unittest from contextlib import redirect_stderr from types import SimpleNamespace +try: + import openpyxl +except ImportError: + openpyxl = None + from salary_lookup import format_entry from tools.convert_salary_excel import ( INDEX_PATTERNS, @@ -32,6 +42,51 @@ class FakeWorksheet: return [SimpleNamespace(value=value) for value in self.rows[row_number - 1]] +@unittest.skipUnless(openpyxl is not None, "openpyxl is not installed") +class BooleanCellCliTests(unittest.TestCase): + def test_excel_logical_cells_are_not_salary_numbers(self): + # Exercise the documented converter CLI with actual Excel boolean cells, + # including partial pairs and numeric 0/1 controls. + with tempfile.TemporaryDirectory() as tmp: + source = Path(tmp) / "salary.xlsx" + target = Path(tmp) / "salary.json" + workbook = openpyxl.Workbook() + sheet = workbook.active + sheet.append(["Company", "City", "Count", "Index", "Verified"]) + sheet.append(["Logical Pair", "Berlin", True, False, True]) + sheet.append(["Logical Count", "Berlin", False, 108.5, False]) + sheet.append(["Logical Index", "Berlin", 12, True, None]) + sheet.append(["Numeric Values", "Berlin", 0, 1, 0]) + sheet.append(["Numeric Count", "Berlin", 1, 0, 1]) + workbook.save(source) + workbook.close() + + result = subprocess.run( + [sys.executable, str(Path(__file__).resolve().parents[1] / + "tools/convert_salary_excel.py"), + str(source), "--output", str(target)], + capture_output=True, text=True, + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + companies = json.loads(target.read_text(encoding="utf-8"))["companies"] + + self.assertEqual(companies[0]["categories"], {}) + self.assertEqual(companies[1]["categories"], { + "all_employees": {"count": None, "index": 108.5}, + }) + self.assertEqual(companies[2]["categories"], { + "all_employees": {"count": 12, "index": None}, + }) + self.assertEqual(companies[3]["categories"], { + "all_employees": {"count": 0, "index": 1.0}, + "verified": {"index": 0.0}, + }) + self.assertEqual(companies[4]["categories"], { + "all_employees": {"count": 1, "index": 0.0}, + "verified": {"index": 1.0}, + }) + + class DetectColumnTypeTests(unittest.TestCase): def test_index_headers_are_not_misclassified_as_count(self): for header in ("Index", "Salary Index", "Engineering Index", "Median salary"): diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index f844552e..728f427c 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -68,6 +68,8 @@ def _finite_float(value): def parse_numeric_cell(value): """Parse numeric Excel values, including localized string cells.""" + if isinstance(value, bool): + raise ValueError("not numeric") if isinstance(value, (int, float)): return _finite_float(value) if not isinstance(value, str):