fix(salary): skip Excel logical cells instead of importing them as numbers (#664)

* fix(salary): skip Excel logical cells instead of importing numbers

* test: skip Excel CLI case when openpyxl is unavailable

* changelog: place the #664 entry under [Unreleased]

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Mads Lorentzen <madslorentzen_17@hotmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
authored and GitHub committed 2026-10-20 21:02:24 +02:00
1 parent 2aadb070f4
commit 4339153f8f
3 files changed
+62

No files matched your search

+5
View File
@@ -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` 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. 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 ### Security
- **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a - **CI no longer leaves the job token in `.git/config` or expands a PR-controlled name into a
+55
View File
@@ -1,8 +1,18 @@
import io import io
import json
from pathlib import Path
import subprocess
import sys
import tempfile
import unittest import unittest
from contextlib import redirect_stderr from contextlib import redirect_stderr
from types import SimpleNamespace from types import SimpleNamespace
try:
import openpyxl
except ImportError:
openpyxl = None
from salary_lookup import format_entry from salary_lookup import format_entry
from tools.convert_salary_excel import ( from tools.convert_salary_excel import (
INDEX_PATTERNS, INDEX_PATTERNS,
@@ -32,6 +42,51 @@ class FakeWorksheet:
return [SimpleNamespace(value=value) for value in self.rows[row_number - 1]] 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): class DetectColumnTypeTests(unittest.TestCase):
def test_index_headers_are_not_misclassified_as_count(self): def test_index_headers_are_not_misclassified_as_count(self):
for header in ("Index", "Salary Index", "Engineering Index", "Median salary"): for header in ("Index", "Salary Index", "Engineering Index", "Median salary"):
+2
View File
@@ -68,6 +68,8 @@ def _finite_float(value):
def parse_numeric_cell(value): def parse_numeric_cell(value):
"""Parse numeric Excel values, including localized string cells.""" """Parse numeric Excel values, including localized string cells."""
if isinstance(value, bool):
raise ValueError("not numeric")
if isinstance(value, (int, float)): if isinstance(value, (int, float)):
return _finite_float(value) return _finite_float(value)
if not isinstance(value, str): if not isinstance(value, str):