From 092982e17e8a12fbe2db478e0de54d6192e00eca Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 24 Oct 2026 23:02:06 -0400 Subject: [PATCH 1/4] Calibrate households' SDLT to the OBR total less corporate SDLT The OBR's SDLT line covers every buyer, but policyengine-uk adds SDLT from non-natural persons separately as corporate_sdlt, so targeting stamp_duty_land_tax at the whole line counted the corporate share twice. Subtract the installed policyengine-uk's corporate statistic for each of the active OBR manifest's years, warn when that statistic is held flat (policyengine-uk before #2322, first released in 2.152.0), and refuse full builds against such a version. A statistic above the OBR line raises SourceError, which the receipts parser does not swallow. Fixes #564 Co-Authored-By: Claude Opus 5.5 --- .../sdlt-target-excludes-corporate.fixed.md | 1 + .../datasets/create_datasets.py | 3 +- policyengine_uk_data/targets/sources/obr.py | 65 ++++++++ .../tests/test_build_environment.py | 23 +++ .../tests/test_obr_receipts_sheet.py | 4 +- .../tests/test_obr_sdlt_target.py | 152 ++++++++++++++++++ .../tests/test_obr_sources.py | 4 + .../tests/test_property_purchased.py | 63 +++++--- .../utils/build_environment.py | 32 +++- 9 files changed, 318 insertions(+), 29 deletions(-) create mode 100644 changelog.d/sdlt-target-excludes-corporate.fixed.md create mode 100644 policyengine_uk_data/tests/test_obr_sdlt_target.py diff --git a/changelog.d/sdlt-target-excludes-corporate.fixed.md b/changelog.d/sdlt-target-excludes-corporate.fixed.md new file mode 100644 index 000000000..038c4efce --- /dev/null +++ b/changelog.d/sdlt-target-excludes-corporate.fixed.md @@ -0,0 +1 @@ +Calibrate households' SDLT (`stamp_duty_land_tax`) to the OBR's SDLT forecast less policyengine-uk's corporate SDLT statistic for the same year, instead of the whole OBR line. policyengine-uk adds SDLT paid by companies and other non-natural persons separately, as `corporate_sdlt`, so calibration counted that share twice: on the 1.58.0 dataset with PolicyEngine/policyengine-uk#2322, 2026-27 SDLT came to £23.29bn (£17.31bn households, £5.98bn corporate) against the OBR's £18.15bn. The 2026-27 household target becomes £12.17bn. Full builds now need a policyengine-uk with #2322, which projects the corporate statistic with the OBR forecast, and stop at the start with an earlier one, which holds it at £3.13bn in every year. diff --git a/policyengine_uk_data/datasets/create_datasets.py b/policyengine_uk_data/datasets/create_datasets.py index 6e047d92b..8779c3c73 100644 --- a/policyengine_uk_data/datasets/create_datasets.py +++ b/policyengine_uk_data/datasets/create_datasets.py @@ -58,7 +58,8 @@ def _materialize_base_year_dataset(dataset, frs_release, uprate_dataset): def main(): """Create enhanced FRS dataset with rich progress tracking.""" try: - assert_local_build_environment() + # TESTING builds (PR CI) skip the full-build checks. + assert_local_build_environment(full_build=os.environ.get("TESTING", "0") != "1") from policyengine_uk.data import UKSingleYearDataset from policyengine_uk_data.datasets.disability_benefits import ( diff --git a/policyengine_uk_data/targets/sources/obr.py b/policyengine_uk_data/targets/sources/obr.py index ff57ff055..b076210fc 100644 --- a/policyengine_uk_data/targets/sources/obr.py +++ b/policyengine_uk_data/targets/sources/obr.py @@ -14,6 +14,7 @@ from policyengine_uk_data.targets.obr_sources import ( ObrManifest, + SourceError, active_manifest, check_coverage, load_workbook, @@ -71,6 +72,68 @@ def _find_row(ws, label: str, col: str = "B", max_row: int = 80) -> int: raise ValueError(f"Row '{label}' not found in sheet") +def corporate_sdlt_receipts(years=None) -> dict[int, float]: + """policyengine-uk's SDLT receipts from non-natural persons, by year. + + These are the statistics `corporate_sdlt` allocates to households: each + household's `shareholding` divides its corporate-sector wealth by the + weighted total, so the weighted sum of `corporate_sdlt` is this statistic + whatever the weights. Read from the installed policyengine-uk, which + projects it from PolicyEngine/policyengine-uk#2322 on. Years are + PolicyEngine years (2025 is 2025-26); the default is the active OBR + manifest's years. + """ + from policyengine_uk.system import system + + statistics = system.parameters.gov.hmrc.stamp_duty.statistics + return { + year: float( + statistics.residential.corporate.revenue(f"{year}-01-01") + + statistics.non_residential.corporate.revenue(f"{year}-01-01") + ) + for year in (active_manifest().years if years is None else years) + } + + +def corporate_sdlt_is_projected(corporate: dict[int, float]) -> bool: + """Whether the corporate SDLT statistic changes over the years given. + + policyengine-uk before PolicyEngine/policyengine-uk#2322 held it at its + 2020-21 value, £3.13bn, in every later year. + """ + return len(set(corporate.values())) > 1 + + +def household_sdlt(obr_sdlt: dict[int, float]) -> dict[int, float]: + """The OBR's SDLT receipts less policyengine-uk's corporate SDLT, by year. + + The OBR's SDLT line covers every buyer. policyengine-uk counts receipts + from non-natural persons (companies, partnerships, collective investment + schemes) separately, as `corporate_sdlt`, so targeting the household + variable `stamp_duty_land_tax` at the whole line counted them twice (#564). + Raises SourceError, which the receipts parser does not catch, if the + statistic exceeds the OBR's line in any year. + """ + corporate = corporate_sdlt_receipts(obr_sdlt) + if len(corporate) > 1 and not corporate_sdlt_is_projected(corporate): + logger.warning( + "OBR receipts: policyengine-uk holds corporate SDLT at £%.2fbn in " + "every year from %d to %d, so it predates " + "PolicyEngine/policyengine-uk#2322 and the household SDLT target " + "overstates households' share. Full builds refuse it.", + next(iter(corporate.values())) / 1e9, + min(corporate), + max(corporate), + ) + household = {year: total - corporate[year] for year, total in obr_sdlt.items()} + negative = sorted(year for year, value in household.items() if value < 0) + if negative: + raise SourceError( + f"policyengine-uk's corporate SDLT exceeds the OBR's SDLT in {negative}" + ) + return household + + def _parse_receipts( wb: openpyxl.Workbook, manifest: ObrManifest | None = None ) -> list[Target]: @@ -136,6 +199,8 @@ def _parse_receipts( try: row_num = _find_row(ws39, label, col="B", max_row=80) values = read_row(ws39, row_num, manifest) + if values and name == "sdlt": + values = household_sdlt(values) if values: recovered.append(label) targets.append( diff --git a/policyengine_uk_data/tests/test_build_environment.py b/policyengine_uk_data/tests/test_build_environment.py index 3f8676453..d6f5e1003 100644 --- a/policyengine_uk_data/tests/test_build_environment.py +++ b/policyengine_uk_data/tests/test_build_environment.py @@ -29,3 +29,26 @@ def test_get_local_build_issues_accepts_supported_environment(): ) == [] ) + + +def test_get_local_build_issues_requires_projected_corporate_sdlt(): + """policyengine-uk before #2322 held corporate SDLT at £3.13bn.""" + issues = get_local_build_issues( + python_version=(3, 13, 7), + variable_names={"num_vehicles"}, + corporate_sdlt=dict.fromkeys(range(2024, 2031), 3.13e9), + ) + + assert len(issues) == 1 + assert "PolicyEngine/policyengine-uk#2322" in issues[0] + + +def test_get_local_build_issues_accepts_projected_corporate_sdlt(): + assert ( + get_local_build_issues( + python_version=(3, 13, 7), + variable_names={"num_vehicles"}, + corporate_sdlt={2024: 4.49e9, 2025: 4.81e9, 2026: 5.98e9}, + ) + == [] + ) diff --git a/policyengine_uk_data/tests/test_obr_receipts_sheet.py b/policyengine_uk_data/tests/test_obr_receipts_sheet.py index 3271d5f42..35481011f 100644 --- a/policyengine_uk_data/tests/test_obr_receipts_sheet.py +++ b/policyengine_uk_data/tests/test_obr_receipts_sheet.py @@ -105,8 +105,10 @@ def _receipts_wb(*, populate: bool) -> openpyxl.Workbook: row = 10 + offset ws[f"B{row}"] = label if populate: + # £100bn a year, above the corporate SDLT statistic the SDLT + # target subtracts. for col in ("D", "E", "F", "G", "H", "I", "J"): - ws[f"{col}{row}"] = 1.0 + ws[f"{col}{row}"] = 100.0 return wb diff --git a/policyengine_uk_data/tests/test_obr_sdlt_target.py b/policyengine_uk_data/tests/test_obr_sdlt_target.py new file mode 100644 index 000000000..aee15d1c3 --- /dev/null +++ b/policyengine_uk_data/tests/test_obr_sdlt_target.py @@ -0,0 +1,152 @@ +"""The OBR SDLT target leaves out the corporate SDLT policyengine-uk adds (#564). + +The OBR's SDLT line covers every buyer. policyengine-uk adds SDLT paid by +non-natural persons as `corporate_sdlt` (its corporate statistic times each +household's shareholding), so household `stamp_duty_land_tax` is targeted at +the OBR line less that statistic. + +Invariants, for every year the OBR forecasts: +- target + corporate statistic = OBR SDLT; +- 0 <= target <= OBR SDLT whenever 0 <= statistic <= OBR SDLT, and a statistic + above the OBR figure is an error rather than a negative target; +- the statistic subtracted is the one `corporate_sdlt` adds. +""" + +import logging +from unittest.mock import patch + +import pytest +from hypothesis import given +from hypothesis import strategies as st + +from policyengine_uk_data.targets.obr_sources import ( + SourceError, + active_manifest, + load_workbook, + read_row, +) +from policyengine_uk_data.targets.sources import obr + +YEARS = active_manifest().years + + +@pytest.fixture(scope="module") +def receipts_workbook(): + """The verified EFO receipts workbook the active manifest names.""" + return load_workbook("receipts") + + +def _sheet(wb, title: str): + (name,) = [n for n in wb.sheetnames if title in str(wb[n]["B2"].value)] + return wb[name] + + +def _row(ws, label: str) -> dict[int, float]: + """A row's values by year, with years from the table's own header.""" + return read_row(ws, obr._find_row(ws, label), active_manifest()) + + +def _stub_statistic(corporate: dict[int, float]): + return patch.object( + obr, + "corporate_sdlt_receipts", + lambda years=None: {year: corporate[year] for year in years}, + ) + + +def test_target_is_obr_sdlt_less_corporate_statistic(receipts_workbook): + obr_sdlt = _row(obr._find_receipts_sheet(receipts_workbook), "Stamp duty land tax") + target = {t.name: t for t in obr._parse_receipts(receipts_workbook)}["obr/sdlt"] + corporate = obr.corporate_sdlt_receipts(YEARS) + + assert target.variable == "stamp_duty_land_tax" + assert sorted(target.values) == YEARS + for year in YEARS: + assert target.values[year] == pytest.approx( + obr_sdlt[year] - corporate[year], rel=1e-12 + ) + + +def test_receipts_sdlt_line_is_the_property_type_total(receipts_workbook): + """The line the statistic is subtracted from is the total of the property + types policyengine-uk projects corporate SDLT with (EFO Table 3.7).""" + by_sector = _sheet(receipts_workbook, "Property transactions taxes") + total = _row(by_sector, "Total SDLT") + assert sorted(total) == YEARS + assert total == pytest.approx( + _row(obr._find_receipts_sheet(receipts_workbook), "Stamp duty land tax") + ) + + +def test_statistic_is_what_corporate_sdlt_adds(): + """A household holding the whole corporate sector pays the statistic.""" + from policyengine_uk import Simulation + + simulation = Simulation( + situation={ + "people": {"adult": {"age": dict.fromkeys(YEARS, 40)}}, + "benunits": {"benunit": {"members": ["adult"]}}, + "households": { + "household": { + "members": ["adult"], + "shareholding": dict.fromkeys(YEARS, 1.0), + } + }, + } + ) + statistic = obr.corporate_sdlt_receipts(YEARS) + for year in YEARS: + assert float(simulation.calculate("corporate_sdlt", year)[0]) == ( + pytest.approx(statistic[year], rel=1e-6) + ) + + +_amounts = st.floats(min_value=0, max_value=1e12, allow_nan=False) + + +@given(st.dictionaries(st.integers(2000, 2040), st.tuples(_amounts, _amounts))) +def test_household_sdlt_is_obr_sdlt_less_statistic(pairs): + obr_sdlt = {year: max(pair) for year, pair in pairs.items()} + corporate = {year: min(pair) for year, pair in pairs.items()} + with _stub_statistic(corporate): + household = obr.household_sdlt(obr_sdlt) + + assert household.keys() == obr_sdlt.keys() + for year, total in obr_sdlt.items(): + assert 0 <= household[year] <= total + assert household[year] + corporate[year] == pytest.approx(total, abs=1e-3) + + +@given( + st.dictionaries(st.integers(2000, 2040), _amounts, min_size=1), + st.floats(min_value=1, max_value=1e9), + st.data(), +) +def test_statistic_above_obr_sdlt_is_an_error(obr_sdlt, excess, data): + year = data.draw(st.sampled_from(sorted(obr_sdlt))) + corporate = {**dict.fromkeys(obr_sdlt, 0.0), year: obr_sdlt[year] + excess} + with _stub_statistic(corporate), pytest.raises(SourceError, match=str(year)): + obr.household_sdlt(obr_sdlt) + + +def test_statistic_above_obr_sdlt_stops_the_receipts_parse(receipts_workbook): + """The error is not caught as a missing row and the target dropped.""" + with ( + _stub_statistic(dict.fromkeys(YEARS, 1e12)), + pytest.raises(SourceError, match="exceeds the OBR's SDLT"), + ): + obr._parse_receipts(receipts_workbook) + + +def test_flat_statistic_warns(caplog): + flat = dict.fromkeys(YEARS, 3.13e9) + with _stub_statistic(flat), caplog.at_level(logging.WARNING): + obr.household_sdlt(dict.fromkeys(YEARS, 20e9)) + assert "PolicyEngine/policyengine-uk#2322" in caplog.text + + +def test_projected_statistic_does_not_warn(caplog): + projected = {year: 4e9 + 1e8 * (year - YEARS[0]) for year in YEARS} + with _stub_statistic(projected), caplog.at_level(logging.WARNING): + obr.household_sdlt(dict.fromkeys(YEARS, 20e9)) + assert caplog.text == "" diff --git a/policyengine_uk_data/tests/test_obr_sources.py b/policyengine_uk_data/tests/test_obr_sources.py index 7a5f495fb..f790aef4b 100644 --- a/policyengine_uk_data/tests/test_obr_sources.py +++ b/policyengine_uk_data/tests/test_obr_sources.py @@ -153,12 +153,16 @@ def test_header_read_years_match_the_fixed_columns_on_march_2026(): for sheet, fixed in _FIXED.items(): ws = books["receipts" if sheet.startswith("3") else "expenditure"][sheet] assert year_columns(ws, 2024, 2030) == fixed, sheet + corporate_sdlt = obr.corporate_sdlt_receipts() for name, (kind, sheet, label) in _ROWS.items(): ws = books[kind][sheet] row = obr._find_row(ws, label, max_row=80) expected = { year: ws[f"{c}{row}"].value * 1e9 for c, year in _FIXED[sheet].items() } + if name == "obr/sdlt": + # Households' SDLT is targeted at the line less corporate SDLT (#564). + expected = {year: v - corporate_sdlt[year] for year, v in expected.items()} assert targets[name].values == pytest.approx(expected), name diff --git a/policyengine_uk_data/tests/test_property_purchased.py b/policyengine_uk_data/tests/test_property_purchased.py index 0c742180c..c9f3165da 100644 --- a/policyengine_uk_data/tests/test_property_purchased.py +++ b/policyengine_uk_data/tests/test_property_purchased.py @@ -11,14 +11,19 @@ https://www.ons.gov.uk/peoplepopulationandcommunity/birthsdeathsandmarriages/families/bulletins/familiesandhouseholds/2024 - Rate: 1.1m / 28.6m = 3.85% -Verification against official SDLT revenue (2024-25): -- Official SDLT: £13.9bn (https://www.gov.uk/government/statistics/uk-stamp-tax-statistics) -- With fix (3.85%): ~£15bn (close to official) -- Without fix (100%): £370bn (26x too high) +Households' SDLT is checked against the OBR's SDLT forecast less the SDLT +policyengine-uk attributes to companies and other non-natural persons +(`corporate_sdlt`), the figure calibration targets (#564). Before the 3.85% +rate, every household was charged SDLT and the total was about £370bn. """ +import os + import pytest +from policyengine_uk_data.targets.obr_sources import load_workbook +from policyengine_uk_data.targets.sources import obr + PROPERTY_PURCHASE_RATE = 0.0385 @@ -62,31 +67,39 @@ def test_property_purchased_has_some_true(baseline): assert true_count > 0, "No households have property_purchased=True" -def test_sdlt_total_reasonable(baseline): - """Test that total SDLT revenue is realistic. +# PR CI builds with TESTING=1 (32 calibration epochs instead of 512), so its +# bound only catches order-of-magnitude errors. Full builds get the strict one. +_SDLT_BOUNDS = (0.25, 4.0) if os.environ.get("TESTING") == "1" else (0.5, 1.5) - Official SDLT revenue (2024-25): £13.9bn - Source: https://www.gov.uk/government/statistics/uk-stamp-tax-statistics - Without fix (100% property_purchased=True): £370bn (26x too high) - With fix (3.85% rate): ~£15bn (close to official) - """ - expected_sdlt = baseline.calculate("expected_sdlt", 2025).values - household_weight = baseline.calculate("household_weight", 2025).values - total_sdlt = (expected_sdlt * household_weight).sum() +def _household_sdlt_target(year: int) -> float: + """The OBR SDLT calibration target: the OBR's SDLT less corporate SDLT.""" + targets = obr._parse_receipts(load_workbook("receipts")) + return {t.name: t for t in targets}["obr/sdlt"].values[year] - # Total SDLT should be within reasonable range of official figures - min_sdlt = 5e9 # £5bn minimum - max_sdlt = 50e9 # £50bn maximum (official is ~£14bn) - assert total_sdlt > min_sdlt, ( - f"Total SDLT £{total_sdlt / 1e9:.1f}bn is too low " - f"(minimum expected: £{min_sdlt / 1e9:.1f}bn)" +def test_household_sdlt_near_target(baseline): + """Households' SDLT is near the OBR's SDLT less corporate SDLT. + + Compared with `stamp_duty_land_tax`, households' own SDLT: `expected_sdlt` + also included `corporate_sdlt` before PolicyEngine/policyengine-uk#2235. + """ + weight = baseline.calculate("household_weight", 2025).values + household = (baseline.calculate("stamp_duty_land_tax", 2025).values * weight).sum() + target = _household_sdlt_target(2025) + low, high = _SDLT_BOUNDS + + assert low < household / target < high, ( + f"Households' SDLT is £{household / 1e9:.2f}bn against a target of " + f"£{target / 1e9:.2f}bn (the OBR's SDLT less corporate SDLT); the " + f"ratio must lie between {low} and {high}." ) - assert total_sdlt < max_sdlt, ( - f"Total SDLT £{total_sdlt / 1e9:.1f}bn is unrealistically high " - f"(maximum expected: £{max_sdlt / 1e9:.1f}bn). " - f"Official SDLT is ~£14bn. " - "This suggests property_purchased may be incorrectly set to True for all households." + +def test_corporate_sdlt_is_the_statistic_the_target_subtracts(baseline): + """With households on target, total SDLT is then the OBR's figure.""" + weight = baseline.calculate("household_weight", 2025).values + corporate = (baseline.calculate("corporate_sdlt", 2025).values * weight).sum() + assert corporate == pytest.approx( + obr.corporate_sdlt_receipts([2025])[2025], rel=1e-6 ) diff --git a/policyengine_uk_data/utils/build_environment.py b/policyengine_uk_data/utils/build_environment.py index aa4f0a5e6..b6e128a50 100644 --- a/policyengine_uk_data/utils/build_environment.py +++ b/policyengine_uk_data/utils/build_environment.py @@ -5,7 +5,14 @@ def get_local_build_issues( python_version: tuple[int, ...] | None = None, variable_names: Iterable[str] | None = None, + corporate_sdlt: dict[int, float] | None = None, ) -> list[str]: + """Problems with the build environment. + + ``corporate_sdlt`` is the resolved policyengine-uk's corporate SDLT + statistic by year (``obr.corporate_sdlt_receipts``). It is checked only + when given, which full builds do. + """ issues: list[str] = [] if python_version is None: @@ -30,11 +37,32 @@ def get_local_build_issues( "PYTHONPATH." ) + if corporate_sdlt is not None: + from policyengine_uk_data.targets.sources.obr import ( + corporate_sdlt_is_projected, + ) + + if not corporate_sdlt_is_projected(corporate_sdlt): + issues.append( + "The resolved policyengine-uk holds corporate SDLT at " + f"£{next(iter(corporate_sdlt.values())) / 1e9:.2f}bn in every " + f"year from {min(corporate_sdlt)} to {max(corporate_sdlt)}. " + "Full builds need a version with " + "PolicyEngine/policyengine-uk#2322, which projects it with the " + "OBR forecast: households' SDLT is calibrated to the OBR total " + "less this statistic, so a stale one overstates it." + ) + return issues -def assert_local_build_environment() -> None: - issues = get_local_build_issues() +def assert_local_build_environment(full_build: bool = False) -> None: + corporate_sdlt = None + if full_build: + from policyengine_uk_data.targets.sources.obr import corporate_sdlt_receipts + + corporate_sdlt = corporate_sdlt_receipts() + issues = get_local_build_issues(corporate_sdlt=corporate_sdlt) if not issues: return From 087e23e9b45173389dad2339359ea53007bb69ad Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sat, 24 Oct 2026 23:11:51 -0400 Subject: [PATCH 2/4] Restate the changelog example on the version 1.58.0 was built with MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The £23.29bn 2026-27 example came from policyengine-uk#2322's head before #2241, which no release matches. Use 1.58.0 under the locked 2.122.2 (households £16.32bn against the OBR's £15.41bn in 2025-26, corporate £3.13bn on top) and the 2025-26 target with 2.152.0 or later. Co-Authored-By: Claude Opus 5.5 --- changelog.d/sdlt-target-excludes-corporate.fixed.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/sdlt-target-excludes-corporate.fixed.md b/changelog.d/sdlt-target-excludes-corporate.fixed.md index 038c4efce..f1c673a69 100644 --- a/changelog.d/sdlt-target-excludes-corporate.fixed.md +++ b/changelog.d/sdlt-target-excludes-corporate.fixed.md @@ -1 +1 @@ -Calibrate households' SDLT (`stamp_duty_land_tax`) to the OBR's SDLT forecast less policyengine-uk's corporate SDLT statistic for the same year, instead of the whole OBR line. policyengine-uk adds SDLT paid by companies and other non-natural persons separately, as `corporate_sdlt`, so calibration counted that share twice: on the 1.58.0 dataset with PolicyEngine/policyengine-uk#2322, 2026-27 SDLT came to £23.29bn (£17.31bn households, £5.98bn corporate) against the OBR's £18.15bn. The 2026-27 household target becomes £12.17bn. Full builds now need a policyengine-uk with #2322, which projects the corporate statistic with the OBR forecast, and stop at the start with an earlier one, which holds it at £3.13bn in every year. +Calibrate households' SDLT (`stamp_duty_land_tax`) to the OBR's SDLT forecast less policyengine-uk's corporate SDLT statistic for the same year, instead of the whole OBR line. policyengine-uk adds SDLT paid by companies and other non-natural persons separately, as `corporate_sdlt`, so calibration counted that share twice: on the 1.58.0 dataset with policyengine-uk 2.122.2, households' SDLT was £16.32bn in 2025-26 against the OBR's £15.41bn for all buyers, and `corporate_sdlt` added £3.13bn on top. With policyengine-uk 2.152.0 or later, which projects the corporate statistic with the OBR forecast (PolicyEngine/policyengine-uk#2322), the 2025-26 household target is £10.60bn. Full builds now need such a version and stop at the start with an earlier one, which holds the statistic at £3.13bn in every year. From d7ccad719e2793be15c8a8363778a5289e38e423 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 25 Oct 2026 02:36:45 -0400 Subject: [PATCH 3/4] Detect pre-#2322 policyengine-uk by its uprating index, not flat values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review r2 of #569: the variation check refused a valid one-year manifest and any projection that happened to be flat, and let a £1 variation pass. #2322 gives both corporate SDLT revenue parameters an uprating index, so check for that (obr.unprojected_corporate_sdlt) in the target warning and the full-build preflight. Also import create_datasets' progress panels before the preflight runs, so its error handler can report a refused build instead of raising UnboundLocalError, and relax Hypothesis's time limit on the two SDLT properties, which failed the too_slow health check after policyengine-uk's imports on a loaded host. Co-Authored-By: Claude Opus 5.5 --- .../datasets/create_datasets.py | 13 ++-- policyengine_uk_data/targets/sources/obr.py | 38 ++++++---- .../tests/test_build_environment.py | 21 +++++- .../tests/test_obr_sdlt_target.py | 74 ++++++++++++++++--- .../utils/build_environment.py | 42 +++++------ 5 files changed, 132 insertions(+), 56 deletions(-) diff --git a/policyengine_uk_data/datasets/create_datasets.py b/policyengine_uk_data/datasets/create_datasets.py index 8779c3c73..0549d599b 100644 --- a/policyengine_uk_data/datasets/create_datasets.py +++ b/policyengine_uk_data/datasets/create_datasets.py @@ -57,6 +57,14 @@ def _materialize_base_year_dataset(dataset, frs_release, uprate_dataset): def main(): """Create enhanced FRS dataset with rich progress tracking.""" + # Imported before the try, whose handler reports any failure with it, + # the preflight's included. + from policyengine_uk_data.utils.progress import ( + ProcessingProgress, + display_success_panel, + display_error_panel, + ) + try: # TESTING builds (PR CI) skip the full-build checks. assert_local_build_environment(full_build=os.environ.get("TESTING", "0") != "1") @@ -68,11 +76,6 @@ def main(): from policyengine_uk_data.datasets.frs import create_frs from policyengine_uk_data.datasets.frs_release import CURRENT_FRS_RELEASE from policyengine_uk_data.storage import STORAGE_FOLDER - from policyengine_uk_data.utils.progress import ( - ProcessingProgress, - display_success_panel, - display_error_panel, - ) from policyengine_uk_data.utils.subsample import subsample_dataset from policyengine_uk_data.utils.uprating import uprate_dataset diff --git a/policyengine_uk_data/targets/sources/obr.py b/policyengine_uk_data/targets/sources/obr.py index b076210fc..be345b505 100644 --- a/policyengine_uk_data/targets/sources/obr.py +++ b/policyengine_uk_data/targets/sources/obr.py @@ -79,9 +79,9 @@ def corporate_sdlt_receipts(years=None) -> dict[int, float]: household's `shareholding` divides its corporate-sector wealth by the weighted total, so the weighted sum of `corporate_sdlt` is this statistic whatever the weights. Read from the installed policyengine-uk, which - projects it from PolicyEngine/policyengine-uk#2322 on. Years are - PolicyEngine years (2025 is 2025-26); the default is the active OBR - manifest's years. + projects it from PolicyEngine/policyengine-uk#2322 on (see + `unprojected_corporate_sdlt`). Years are PolicyEngine years (2025 is + 2025-26); the default is the active OBR manifest's years. """ from policyengine_uk.system import system @@ -95,13 +95,23 @@ def corporate_sdlt_receipts(years=None) -> dict[int, float]: } -def corporate_sdlt_is_projected(corporate: dict[int, float]) -> bool: - """Whether the corporate SDLT statistic changes over the years given. +def unprojected_corporate_sdlt() -> list[str]: + """The corporate SDLT revenue parameters policyengine-uk does not project. - policyengine-uk before PolicyEngine/policyengine-uk#2322 held it at its - 2020-21 value, £3.13bn, in every later year. + PolicyEngine/policyengine-uk#2322 (first released in 2.152.0) gives each + one an uprating index, the OBR's SDLT receipts for its property type, so + that load-time uprating projects it past HMRC's last outturn. Earlier + versions have no index and carry the 2020-21 figures forward unchanged, + £3.13bn in all, so the check is on the index, not on the values. """ - return len(set(corporate.values())) > 1 + from policyengine_uk.system import system + + statistics = system.parameters.gov.hmrc.stamp_duty.statistics + return [ + f"gov.hmrc.stamp_duty.statistics.{kind}.corporate.revenue" + for kind in ("residential", "non_residential") + if not getattr(statistics, kind).corporate.revenue.metadata.get("uprating") + ] def household_sdlt(obr_sdlt: dict[int, float]) -> dict[int, float]: @@ -114,17 +124,15 @@ def household_sdlt(obr_sdlt: dict[int, float]) -> dict[int, float]: Raises SourceError, which the receipts parser does not catch, if the statistic exceeds the OBR's line in any year. """ - corporate = corporate_sdlt_receipts(obr_sdlt) - if len(corporate) > 1 and not corporate_sdlt_is_projected(corporate): + unprojected = unprojected_corporate_sdlt() + if unprojected: logger.warning( - "OBR receipts: policyengine-uk holds corporate SDLT at £%.2fbn in " - "every year from %d to %d, so it predates " + "OBR receipts: policyengine-uk does not project %s, so it predates " "PolicyEngine/policyengine-uk#2322 and the household SDLT target " "overstates households' share. Full builds refuse it.", - next(iter(corporate.values())) / 1e9, - min(corporate), - max(corporate), + " or ".join(unprojected), ) + corporate = corporate_sdlt_receipts(obr_sdlt) household = {year: total - corporate[year] for year, total in obr_sdlt.items()} negative = sorted(year for year, value in household.items() if value < 0) if negative: diff --git a/policyengine_uk_data/tests/test_build_environment.py b/policyengine_uk_data/tests/test_build_environment.py index d6f5e1003..8d31f717b 100644 --- a/policyengine_uk_data/tests/test_build_environment.py +++ b/policyengine_uk_data/tests/test_build_environment.py @@ -1,3 +1,5 @@ +import pytest + from policyengine_uk_data.utils.build_environment import get_local_build_issues @@ -36,11 +38,14 @@ def test_get_local_build_issues_requires_projected_corporate_sdlt(): issues = get_local_build_issues( python_version=(3, 13, 7), variable_names={"num_vehicles"}, - corporate_sdlt=dict.fromkeys(range(2024, 2031), 3.13e9), + unprojected_corporate_sdlt=[ + "gov.hmrc.stamp_duty.statistics.residential.corporate.revenue" + ], ) assert len(issues) == 1 assert "PolicyEngine/policyengine-uk#2322" in issues[0] + assert "statistics.residential.corporate.revenue" in issues[0] def test_get_local_build_issues_accepts_projected_corporate_sdlt(): @@ -48,7 +53,19 @@ def test_get_local_build_issues_accepts_projected_corporate_sdlt(): get_local_build_issues( python_version=(3, 13, 7), variable_names={"num_vehicles"}, - corporate_sdlt={2024: 4.49e9, 2025: 4.81e9, 2026: 5.98e9}, + unprojected_corporate_sdlt=[], ) == [] ) + + +def test_a_refused_build_reports_the_preflight_error(monkeypatch): + """The build's error handler can report a failure of the preflight itself.""" + from policyengine_uk_data.datasets import create_datasets + + def refuse(full_build=False): + raise RuntimeError("preflight refused") + + monkeypatch.setattr(create_datasets, "assert_local_build_environment", refuse) + with pytest.raises(RuntimeError, match="preflight refused"): + create_datasets.main() diff --git a/policyengine_uk_data/tests/test_obr_sdlt_target.py b/policyengine_uk_data/tests/test_obr_sdlt_target.py index aee15d1c3..f5dc71418 100644 --- a/policyengine_uk_data/tests/test_obr_sdlt_target.py +++ b/policyengine_uk_data/tests/test_obr_sdlt_target.py @@ -9,14 +9,18 @@ - target + corporate statistic = OBR SDLT; - 0 <= target <= OBR SDLT whenever 0 <= statistic <= OBR SDLT, and a statistic above the OBR figure is an error rather than a negative target; -- the statistic subtracted is the one `corporate_sdlt` adds. +- the statistic subtracted is the one `corporate_sdlt` adds; +- the projection check (an uprating index on each statistic) agrees with the + installed values, which vary only when they are projected. """ +import itertools import logging +from contextlib import ExitStack, contextmanager from unittest.mock import patch import pytest -from hypothesis import given +from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st from policyengine_uk_data.targets.obr_sources import ( @@ -46,12 +50,18 @@ def _row(ws, label: str) -> dict[int, float]: return read_row(ws, obr._find_row(ws, label), active_manifest()) -def _stub_statistic(corporate: dict[int, float]): - return patch.object( - obr, - "corporate_sdlt_receipts", - lambda years=None: {year: corporate[year] for year in years}, - ) +@contextmanager +def _stub_statistic(corporate: dict[int, float], unprojected: tuple = ()): + """Stand in for policyengine-uk's statistic and its projection check.""" + with ( + patch.object( + obr, + "corporate_sdlt_receipts", + lambda years=None: {year: corporate[year] for year in years}, + ), + patch.object(obr, "unprojected_corporate_sdlt", lambda: list(unprojected)), + ): + yield def test_target_is_obr_sdlt_less_corporate_statistic(receipts_workbook): @@ -102,8 +112,12 @@ def test_statistic_is_what_corporate_sdlt_adds(): _amounts = st.floats(min_value=0, max_value=1e12, allow_nan=False) +# Generation can pass Hypothesis's time limit after policyengine-uk's imports +# on a loaded machine; the properties do not depend on timing. +relaxed = settings(deadline=None, suppress_health_check=[HealthCheck.too_slow]) +@relaxed @given(st.dictionaries(st.integers(2000, 2040), st.tuples(_amounts, _amounts))) def test_household_sdlt_is_obr_sdlt_less_statistic(pairs): obr_sdlt = {year: max(pair) for year, pair in pairs.items()} @@ -117,6 +131,7 @@ def test_household_sdlt_is_obr_sdlt_less_statistic(pairs): assert household[year] + corporate[year] == pytest.approx(total, abs=1e-3) +@relaxed @given( st.dictionaries(st.integers(2000, 2040), _amounts, min_size=1), st.floats(min_value=1, max_value=1e9), @@ -138,15 +153,50 @@ def test_statistic_above_obr_sdlt_stops_the_receipts_parse(receipts_workbook): obr._parse_receipts(receipts_workbook) -def test_flat_statistic_warns(caplog): +def test_unprojected_statistic_warns(caplog): flat = dict.fromkeys(YEARS, 3.13e9) - with _stub_statistic(flat), caplog.at_level(logging.WARNING): + unprojected = ("gov.hmrc.stamp_duty.statistics.residential.corporate.revenue",) + with _stub_statistic(flat, unprojected), caplog.at_level(logging.WARNING): obr.household_sdlt(dict.fromkeys(YEARS, 20e9)) assert "PolicyEngine/policyengine-uk#2322" in caplog.text + assert unprojected[0] in caplog.text def test_projected_statistic_does_not_warn(caplog): - projected = {year: 4e9 + 1e8 * (year - YEARS[0]) for year in YEARS} - with _stub_statistic(projected), caplog.at_level(logging.WARNING): + """Even when equal in every year, as a one-year manifest's always is.""" + with _stub_statistic(dict.fromkeys(YEARS, 4e9)), caplog.at_level(logging.WARNING): obr.household_sdlt(dict.fromkeys(YEARS, 20e9)) assert caplog.text == "" + + +@pytest.mark.parametrize("uprated", itertools.product([False, True], repeat=2)) +def test_projection_check_reads_each_statistics_uprating_index(uprated): + """Whatever the installed version, a statistic counts as projected exactly + when its parameter names an uprating index.""" + from policyengine_uk.system import system + + statistics = system.parameters.gov.hmrc.stamp_duty.statistics + expected = [] + with ExitStack() as stack: + for kind, on in zip(("residential", "non_residential"), uprated): + metadata = getattr(statistics, kind).corporate.revenue.metadata + stack.enter_context(patch.dict(metadata)) + metadata.pop("uprating", None) + if on: + metadata["uprating"] = ( + f"gov.economic_assumptions.indices.obr.stamp_duty_land_tax.{kind}" + ) + else: + expected.append( + f"gov.hmrc.stamp_duty.statistics.{kind}.corporate.revenue" + ) + assert obr.unprojected_corporate_sdlt() == expected + + +def test_projection_check_agrees_with_the_installed_values(): + """policyengine-uk either uprates both statistics, which then vary over + the manifest years, or neither, which then stay at their 2020-21 values.""" + unprojected = obr.unprojected_corporate_sdlt() + values = set(obr.corporate_sdlt_receipts().values()) + assert len(unprojected) in (0, 2) + assert (not unprojected) == (len(values) > 1) diff --git a/policyengine_uk_data/utils/build_environment.py b/policyengine_uk_data/utils/build_environment.py index b6e128a50..3f1b1d0de 100644 --- a/policyengine_uk_data/utils/build_environment.py +++ b/policyengine_uk_data/utils/build_environment.py @@ -5,13 +5,14 @@ def get_local_build_issues( python_version: tuple[int, ...] | None = None, variable_names: Iterable[str] | None = None, - corporate_sdlt: dict[int, float] | None = None, + unprojected_corporate_sdlt: Iterable[str] | None = None, ) -> list[str]: """Problems with the build environment. - ``corporate_sdlt`` is the resolved policyengine-uk's corporate SDLT - statistic by year (``obr.corporate_sdlt_receipts``). It is checked only - when given, which full builds do. + ``unprojected_corporate_sdlt`` lists the corporate SDLT statistics the + resolved policyengine-uk does not project + (``obr.unprojected_corporate_sdlt``). It is checked only when given, + which full builds do. """ issues: list[str] = [] @@ -37,32 +38,29 @@ def get_local_build_issues( "PYTHONPATH." ) - if corporate_sdlt is not None: - from policyengine_uk_data.targets.sources.obr import ( - corporate_sdlt_is_projected, + unprojected = list(unprojected_corporate_sdlt or []) + if unprojected: + issues.append( + "The resolved policyengine-uk does not project " + f"{' or '.join(unprojected)}: it carries the last outturn " + "forward. Full builds need policyengine-uk 2.152.0 or later " + "(PolicyEngine/policyengine-uk#2322), which uprates corporate SDLT " + "with the OBR forecast: households' SDLT is calibrated to the OBR " + "total less this statistic, so a stale one overstates it." ) - if not corporate_sdlt_is_projected(corporate_sdlt): - issues.append( - "The resolved policyengine-uk holds corporate SDLT at " - f"£{next(iter(corporate_sdlt.values())) / 1e9:.2f}bn in every " - f"year from {min(corporate_sdlt)} to {max(corporate_sdlt)}. " - "Full builds need a version with " - "PolicyEngine/policyengine-uk#2322, which projects it with the " - "OBR forecast: households' SDLT is calibrated to the OBR total " - "less this statistic, so a stale one overstates it." - ) - return issues def assert_local_build_environment(full_build: bool = False) -> None: - corporate_sdlt = None + unprojected = None if full_build: - from policyengine_uk_data.targets.sources.obr import corporate_sdlt_receipts + from policyengine_uk_data.targets.sources.obr import ( + unprojected_corporate_sdlt, + ) - corporate_sdlt = corporate_sdlt_receipts() - issues = get_local_build_issues(corporate_sdlt=corporate_sdlt) + unprojected = unprojected_corporate_sdlt() + issues = get_local_build_issues(unprojected_corporate_sdlt=unprojected) if not issues: return From 6d2de797db144c0bf7cb9c5f3974941f6eecd311 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 25 Oct 2026 06:23:42 -0400 Subject: [PATCH 4/4] Pin the installed-version projection test to explicit release facts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review r3 of #569: the test assumed projected statistics vary and that a version uprates both or neither, which a flat projection or a partial one would break. Expect both uprated from 2.152.0 (#2322) and, before it, neither, with £3.13bn in every year. Also qualify "whatever the weights" in corporate_sdlt_receipts: shareholding needs a non-zero weighted total. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk_data/targets/sources/obr.py | 11 ++++---- .../tests/test_obr_sdlt_target.py | 27 +++++++++++++------ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/policyengine_uk_data/targets/sources/obr.py b/policyengine_uk_data/targets/sources/obr.py index be345b505..bfce3c1b1 100644 --- a/policyengine_uk_data/targets/sources/obr.py +++ b/policyengine_uk_data/targets/sources/obr.py @@ -77,11 +77,12 @@ def corporate_sdlt_receipts(years=None) -> dict[int, float]: These are the statistics `corporate_sdlt` allocates to households: each household's `shareholding` divides its corporate-sector wealth by the - weighted total, so the weighted sum of `corporate_sdlt` is this statistic - whatever the weights. Read from the installed policyengine-uk, which - projects it from PolicyEngine/policyengine-uk#2322 on (see - `unprojected_corporate_sdlt`). Years are PolicyEngine years (2025 is - 2025-26); the default is the active OBR manifest's years. + weighted total, so while that total is not zero the weighted sum of + `corporate_sdlt` is this statistic whatever the weights. Read from the + installed policyengine-uk, which projects it from + PolicyEngine/policyengine-uk#2322 on (see `unprojected_corporate_sdlt`). + Years are PolicyEngine years (2025 is 2025-26); the default is the active + OBR manifest's years. """ from policyengine_uk.system import system diff --git a/policyengine_uk_data/tests/test_obr_sdlt_target.py b/policyengine_uk_data/tests/test_obr_sdlt_target.py index f5dc71418..ca24140ca 100644 --- a/policyengine_uk_data/tests/test_obr_sdlt_target.py +++ b/policyengine_uk_data/tests/test_obr_sdlt_target.py @@ -10,10 +10,12 @@ - 0 <= target <= OBR SDLT whenever 0 <= statistic <= OBR SDLT, and a statistic above the OBR figure is an error rather than a negative target; - the statistic subtracted is the one `corporate_sdlt` adds; -- the projection check (an uprating index on each statistic) agrees with the - installed values, which vary only when they are projected. +- the projection check reads each statistic's uprating index, and on the + installed policyengine-uk finds both uprated from 2.152.0 (#2322) and + neither before. """ +import importlib.metadata import itertools import logging from contextlib import ExitStack, contextmanager @@ -22,6 +24,7 @@ import pytest from hypothesis import HealthCheck, given, settings from hypothesis import strategies as st +from packaging.version import Version from policyengine_uk_data.targets.obr_sources import ( SourceError, @@ -193,10 +196,18 @@ def test_projection_check_reads_each_statistics_uprating_index(uprated): assert obr.unprojected_corporate_sdlt() == expected -def test_projection_check_agrees_with_the_installed_values(): - """policyengine-uk either uprates both statistics, which then vary over - the manifest years, or neither, which then stay at their 2020-21 values.""" +def test_projection_check_on_the_installed_policyengine_uk(): + """policyengine-uk 2.152.0 and later (#2322) uprate both statistics. The + earlier versions uk-data can lock uprate neither and hold the 2020-21 + figures, £3.13bn in all, in every later year.""" + version = Version(importlib.metadata.version("policyengine-uk")) unprojected = obr.unprojected_corporate_sdlt() - values = set(obr.corporate_sdlt_receipts().values()) - assert len(unprojected) in (0, 2) - assert (not unprojected) == (len(values) > 1) + if version >= Version("2.152.0"): + assert unprojected == [] + else: + assert unprojected == [ + f"gov.hmrc.stamp_duty.statistics.{kind}.corporate.revenue" + for kind in ("residential", "non_residential") + ] + for value in obr.corporate_sdlt_receipts().values(): + assert value == pytest.approx(3.13e9)