diff --git a/CHANGELOG.md b/CHANGELOG.md index 2476e3bd..ec8db9f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -102,6 +102,12 @@ per-file diff commands. longer paired with its count column and `salary_lookup.py` printed the label across two lines. Header whitespace runs now collapse to one space, so a wrapped header converts like its one-line spelling. +- **Salary workbook conversion skips prose notes tabs** (#644, `tools/convert_salary_excel.py`, + `tests/test_salary_notes_sheet_cli.py`) - a "Notes" tab mentioning "company" in a sentence was + taken as a header row by the fallback pass, and every following sentence was emitted as a company + in `salary_data.json` and `--list-all`. A fallback-detected header with no salary column beside it + now skips the sheet with a warning; strictly detected headers are unchanged. + ## [1.7.3] - 2026-10-19 ### Added diff --git a/tests/test_salary_notes_sheet_cli.py b/tests/test_salary_notes_sheet_cli.py new file mode 100644 index 00000000..3f184884 --- /dev/null +++ b/tests/test_salary_notes_sheet_cli.py @@ -0,0 +1,77 @@ +"""A workbook's prose notes tab must not be read as a company table.""" +import json +from pathlib import Path +import shutil +import subprocess +import sys +from tempfile import TemporaryDirectory +import unittest + +try: + import openpyxl +except ImportError: + openpyxl = None + +REPO = Path(__file__).resolve().parent.parent + +NOTES = [ + ["About this data"], + ["Each row is one company in the 2025 survey."], + ["Indexes are relative to the national median."], + ["Categories with fewer than five respondents are suppressed."], +] + + +@unittest.skipUnless(openpyxl, "requires optional openpyxl (installed in CI)") +class NotesSheetCLI(unittest.TestCase): + def setUp(self): + directory = TemporaryDirectory() + self.addCleanup(directory.cleanup) + self.root = Path(directory.name) + shutil.copyfile(REPO / "salary_lookup.py", self.root / "salary_lookup.py") + self.output = self.root / "salary_data.json" + + def convert(self, with_data): + workbook = self.root / "salary.xlsx" + book = openpyxl.Workbook() + try: + sheet = book.active + if with_data: + sheet.title = "Data" + sheet.append(["Company", "City", "Count", "Index"]) + sheet.append(["Acme", "Aarhus", 10, 105]) + sheet = book.create_sheet("Notes") + else: + sheet.title = "Notes" + for line in NOTES: + sheet.append(line) + book.save(workbook) + finally: + book.close() + return subprocess.run( + [sys.executable, str(REPO / "tools" / "convert_salary_excel.py"), str(workbook), + "--output", str(self.output)], + cwd=self.root, capture_output=True, text=True, + ) + + def test_notes_tab_sentences_are_not_listed_as_companies(self): + result = self.convert(with_data=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn("Skipping sheet 'Notes'", result.stderr) + companies = json.loads(self.output.read_text(encoding="utf-8"))["companies"] + self.assertEqual([entry["company"] for entry in companies], ["Acme"]) + listed = subprocess.run( + [sys.executable, "salary_lookup.py", "--list-all"], + cwd=self.root, capture_output=True, text=True, + ) + self.assertEqual(listed.stdout.splitlines(), ["Acme (Aarhus)"]) + + def test_notes_only_workbook_fails_without_writing_output(self): + result = self.convert(with_data=False) + self.assertEqual(result.returncode, 1, result.stdout + result.stderr) + self.assertIn("No data could be parsed", result.stderr) + self.assertFalse(self.output.exists()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/convert_salary_excel.py b/tools/convert_salary_excel.py index 065365a8..11362267 100644 --- a/tools/convert_salary_excel.py +++ b/tools/convert_salary_excel.py @@ -162,6 +162,7 @@ def parse_sheet(ws, sheet_label=None): return [str(cell.value).strip() for cell in row if cell.value] header_row = None + corroborated = True for row_idx, row in enumerate(rows, start=1): cell_texts = _cell_texts(row) company_idxs = {i for i, t in enumerate(cell_texts) if header_matches(t, COMPANY_PATTERNS)} @@ -177,6 +178,7 @@ def parse_sheet(ws, sheet_label=None): break if header_row is None: + corroborated = False for row_idx, row in enumerate(rows, start=1): if any(header_matches(t, COMPANY_PATTERNS) for t in _cell_texts(row)): header_row = row_idx @@ -282,6 +284,18 @@ def parse_sheet(ws, sheet_label=None): for col_idx, col_header in untyped_cols: categories.append({"name": col_header.lower().replace(" ", "_"), "value_col": col_idx}) + if not categories and not corroborated: + # The fallback pass matched a lone company word and no salary column + # sits beside it: this is prose (a "Notes" or "About the data" tab + # mentioning "company"), not a table. Reading it would turn every + # sentence below that line into a company entry. + print( + f"Warning: Skipping sheet '{ws.title}': no table header found " + "(a company word appears only in text, with no salary columns).", + file=sys.stderr, + ) + return [] + if not categories: print( f"Warning: No salary data columns detected in sheet '{ws.title}' "