From 9fc0e299e9ba1bfe30cc85c80933b56768acddc1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 5 Oct 2026 16:55:28 -0400 Subject: [PATCH 1/5] Take SPI Scottish taxpayer status from SCOT_TXP; keep unknown regions simulable create_spi labels GORCODE 13 (address abroad), 14 (address unknown) and -1 (composite records) UNKNOWN. policyengine-uk 2.104.5 (PolicyEngine/policyengine-uk#1985) uprates their rent by the UK-wide index, so the dataset now simulates as built. Scottish taxpayer status comes from HMRC's SCOT_TXP flag rather than the region: the two disagree for 1,394 GORCODE-11 records and 606 Scottish taxpayers elsewhere on the 2022-23 tape. load_spi_dataset relabels UNKNOWN as SOUTH_EAST only on releases before 2.104.5, and ensure_spi_dataset rebuilds a cached H5 without the flag. Co-Authored-By: Claude Opus 5.5 --- changelog.d/spi-unknown-region.fixed.md | 1 + policyengine_uk_data/datasets/spi.py | 39 +++- .../tests/test_spi_allowance_deductions.py | 1 + policyengine_uk_data/tests/test_spi_build.py | 175 +++++++++++++++++- .../utils/incomes_projection.py | 18 +- 5 files changed, 217 insertions(+), 17 deletions(-) create mode 100644 changelog.d/spi-unknown-region.fixed.md diff --git a/changelog.d/spi-unknown-region.fixed.md b/changelog.d/spi-unknown-region.fixed.md new file mode 100644 index 000000000..a8a4b44cf --- /dev/null +++ b/changelog.d/spi-unknown-region.fixed.md @@ -0,0 +1 @@ +Take Scottish taxpayer status in the SPI dataset from HMRC's `SCOT_TXP` flag rather than the region code. `GORCODE` 13, 14 and -1 (address abroad, address unknown, composite records) stay `UNKNOWN`, which policyengine-uk 2.104.5 and later can simulate; `load_spi_dataset` relabels them `SOUTH_EAST` only for older releases, and rebuilds a cached SPI H5 that predates the flag. diff --git a/policyengine_uk_data/datasets/spi.py b/policyengine_uk_data/datasets/spi.py index e22be675c..a21ddb14f 100644 --- a/policyengine_uk_data/datasets/spi.py +++ b/policyengine_uk_data/datasets/spi.py @@ -1,3 +1,6 @@ +import re +from importlib.metadata import version + from policyengine_uk_data.storage import STORAGE_FOLDER import pandas as pd import numpy as np @@ -23,10 +26,11 @@ 7: (74, 90), } -# SPI GORCODE → policyengine-uk region enum. -# NB the SPI codebook does not include a "region unknown" code; we surface -# unknown codes explicitly rather than silently mapping them to SOUTH_EAST -# (which the previous implementation did, distorting regional income totals). +# SPI GORCODE → policyengine-uk region enum. GORCODE also takes 13 ("Address +# abroad"), 14 ("Address unknown or not available") and -1 (composite +# records). None of those is a UK region, so they become "UNKNOWN" rather +# than SOUTH_EAST (which the previous implementation used, distorting +# regional income totals). REGION_MAP = { 1: "NORTH_EAST", 2: "NORTH_WEST", @@ -42,6 +46,16 @@ 12: "NORTHERN_IRELAND", } +# First policyengine-uk release that can simulate Region.UNKNOWN: it uprates +# their rent by the UK-wide index (PolicyEngine/policyengine-uk#1985). +UNKNOWN_REGION_MODEL_VERSION = (2, 104, 5) + + +def model_simulates_unknown_region() -> bool: + """Whether the installed policyengine-uk can simulate Region.UNKNOWN.""" + release = re.match(r"\d+(\.\d+)*", version("policyengine-uk")).group() + return tuple(map(int, release.split("."))) >= UNKNOWN_REGION_MODEL_VERSION + def _get_marriage_allowance(fiscal_year: int) -> float: """Return the maximum Marriage Allowance transfer for the given UK fiscal @@ -98,10 +112,10 @@ def create_spi( existing call sites don't break. seed: Seed for the random age imputation. Fixed by default so builds are deterministic. - unknown_region: Fallback region label for SPI GORCODE values outside - the documented 1-12 range. Defaults to ``"UNKNOWN"`` so regional - totals are not silently distorted; pass ``"SOUTH_EAST"`` to - reproduce legacy behaviour if needed. + unknown_region: Region label for SPI GORCODE values outside 1-12 + (address abroad, address unknown, composite records). Defaults to + ``"UNKNOWN"`` so regional totals are not silently distorted; pass + ``"SOUTH_EAST"`` to reproduce legacy behaviour if needed. """ df = pd.read_csv(spi_data_file_path, delimiter="\t") rng = np.random.default_rng(seed) @@ -119,6 +133,15 @@ def create_spi( person["dividend_income"] = df.DIVIDENDS person["gift_aid"] = df.GIFTAID household["region"] = df.GORCODE.map(REGION_MAP).fillna(unknown_region) + # GORCODE is the address at the end of the tax year; SCOT_TXP marks + # records HMRC taxed under the Scottish system for the year. The two + # disagree for some records and SCOT_TXP is also set on records with no + # UK region, so SCOT_TXP decides the rates. HMRC documents it as "." or 1; + # the 2022-23 tape holds 0 or 1. WELSH_TXP is not used: policyengine-uk + # has no separate Welsh rates. + person["pays_scottish_income_tax"] = ( + pd.to_numeric(df.SCOT_TXP, errors="coerce") == 1 + ) household["rent"] = 0 household["tenure_type"] = "OWNED_OUTRIGHT" household["council_tax"] = 0 diff --git a/policyengine_uk_data/tests/test_spi_allowance_deductions.py b/policyengine_uk_data/tests/test_spi_allowance_deductions.py index f635bb1f6..f40892905 100644 --- a/policyengine_uk_data/tests/test_spi_allowance_deductions.py +++ b/policyengine_uk_data/tests/test_spi_allowance_deductions.py @@ -10,6 +10,7 @@ def test_spi_overrides_allowance_deductions_not_policy_parameters(tmp_path): "DIVIDENDS": 0, "GIFTAID": 0, "GORCODE": 7, + "SCOT_TXP": 0, "INCBBS": 0, "INCPROP": 1_000, "PAY": 0, diff --git a/policyengine_uk_data/tests/test_spi_build.py b/policyengine_uk_data/tests/test_spi_build.py index efb37f810..14c4a5c41 100644 --- a/policyengine_uk_data/tests/test_spi_build.py +++ b/policyengine_uk_data/tests/test_spi_build.py @@ -23,6 +23,7 @@ import numpy as np import pandas as pd import pytest +from policyengine_core.errors import ParameterNotFoundError if importlib.util.find_spec("policyengine_uk") is None: pytest.skip( @@ -30,6 +31,8 @@ allow_module_level=True, ) +from policyengine_uk_data.datasets.spi import model_simulates_unknown_region + SPI_COLUMNS = [ "SEX", @@ -38,6 +41,7 @@ "DIVIDENDS", "GIFTAID", "GORCODE", + "SCOT_TXP", "INCBBS", "INCPROP", "PAY", @@ -356,7 +360,17 @@ def save(self, path): assert dataset_path.read_text() == "rebuilt h5" -def test_income_projection_loads_local_h5_dataset(monkeypatch): +@pytest.mark.parametrize( + "model_simulates, expected", + [ + (False, ["SOUTH_EAST", "LONDON", "SOUTH_EAST"]), + (True, ["UNKNOWN", "LONDON", "SOUTH_EAST"]), + ], +) +def test_income_projection_loads_local_h5_dataset( + monkeypatch, model_simulates, expected +): + """UNKNOWN becomes SOUTH_EAST only for a model that cannot simulate it.""" from policyengine_uk_data.utils import incomes_projection calls = {} @@ -374,16 +388,55 @@ def __init__(self, path): lambda: "/tmp/spi_2022_23.h5", ) monkeypatch.setattr(incomes_projection, "UKSingleYearDataset", FakeDataset) + monkeypatch.setattr( + incomes_projection, + "model_simulates_unknown_region", + lambda: model_simulates, + ) dataset = incomes_projection.load_spi_dataset() assert isinstance(dataset, FakeDataset) assert calls == {"path": "/tmp/spi_2022_23.h5"} - assert dataset.household["region"].tolist() == [ - "SOUTH_EAST", - "LONDON", - "SOUTH_EAST", - ] + assert dataset.household["region"].tolist() == expected + + +def test_income_projection_rebuilds_spi_dataset_without_scottish_flag( + tmp_path, monkeypatch +): + """A cached H5 from before create_spi read SCOT_TXP is rebuilt; a current + one is reused.""" + from policyengine_uk_data.datasets.spi import create_spi + from policyengine_uk_data.utils import incomes_projection + + tab_dir = tmp_path / "spi_2022_23" + tab_dir.mkdir() + tab = tab_dir / "put2223uk.tab" + _write_fake_spi(tab, gor_values=(11, 13), maind_values=(0, 0)) + dataset_path = tmp_path / "spi_2022_23.h5" + stale = create_spi(tab, 2022) + stale.person = stale.person.drop(columns="pays_scottish_income_tax") + stale.save(dataset_path) + + builds = [] + + def counting_create_spi(path, fiscal_year): + builds.append(fiscal_year) + return create_spi(path, fiscal_year) + + monkeypatch.setattr(incomes_projection, "STORAGE_FOLDER", tmp_path) + monkeypatch.setattr(incomes_projection, "SPI_RELEASE_NAME", "spi_2022_23") + monkeypatch.setattr(incomes_projection, "SPI_TAB_FILENAME", "put2223uk.tab") + monkeypatch.setattr(incomes_projection, "SPI_H5_FILENAME", "spi_2022_23.h5") + monkeypatch.setattr(incomes_projection, "SPI_FISCAL_YEAR", 2022) + monkeypatch.setattr(incomes_projection, "create_spi", counting_create_spi) + + assert not incomes_projection._has_scottish_taxpayer_flag(dataset_path) + assert incomes_projection.ensure_spi_dataset() == str(dataset_path) + assert builds == [2022] + assert incomes_projection._has_scottish_taxpayer_flag(dataset_path) + assert incomes_projection.ensure_spi_dataset() == str(dataset_path) + assert builds == [2022] def test_income_model_cache_rejects_stale_spi_release(tmp_path, monkeypatch): @@ -467,3 +520,113 @@ def test_income_model_cache_accepts_current_spi_release(tmp_path, monkeypatch): ) assert income_module.create_income_model().metadata == current_metadata + + +def _set_spi_columns(path, **columns): + df = pd.read_csv(path, sep="\t") + for col, values in columns.items(): + df[col] = list(values) + df.to_csv(path, sep="\t", index=False) + + +@pytest.mark.parametrize("not_scottish", [0, ".", ""]) +def test_create_spi_region_and_scottish_taxpayer_invariants(tmp_path, not_scottish): + """For every GORCODE (documented 1-14, composite -1, undocumented 99) and + SCOT_TXP value, the region follows GORCODE alone and is always a Region + member, and Scottish taxpayer status follows SCOT_TXP alone. HMRC + documents "not a Scottish taxpayer" as "."; the 2022-23 tape writes 0. + """ + from itertools import product + + from policyengine_uk.variables.household.demographic.geography import Region + + from policyengine_uk_data.datasets.spi import REGION_MAP, create_spi + + cases = list(product([-1, *range(1, 15), 99], (not_scottish, 1))) + gor = [g for g, _ in cases] + scot = [s for _, s in cases] + tab = tmp_path / "spi.tab" + _write_fake_spi(tab, gor_values=gor, maind_values=[0] * len(cases)) + _set_spi_columns(tab, SCOT_TXP=scot) + + ds = create_spi(tab, 2022) + + regions = ds.household["region"].tolist() + assert regions == [REGION_MAP.get(g, "UNKNOWN") for g in gor] + assert set(regions) <= {region.name for region in Region} + assert ds.person["pays_scottish_income_tax"].tolist() == [s == 1 for s in scot] + # Address abroad, address unknown and composite records stay UNKNOWN even + # when they are Scottish taxpayers. + assert {r for r, g in zip(regions, gor) if g in (-1, 13, 14)} == {"UNKNOWN"} + + +def test_create_spi_scottish_taxpayer_status_survives_h5_round_trip(tmp_path): + from policyengine_uk.data import UKSingleYearDataset + + from policyengine_uk_data.datasets.spi import create_spi + + tab = tmp_path / "spi.tab" + _write_fake_spi(tab, gor_values=(13, 11, 7), maind_values=(0, 0, 0)) + _set_spi_columns(tab, SCOT_TXP=(1, 0, 1)) + ds = create_spi(tab, 2022) + ds.save(tmp_path / "spi.h5") + + loaded = UKSingleYearDataset(str(tmp_path / "spi.h5")) + + assert loaded.person["pays_scottish_income_tax"].tolist() == [True, False, True] + assert loaded.household["region"].tolist() == ["UNKNOWN", "SCOTLAND", "LONDON"] + + +# GORCODE, SCOT_TXP: abroad, unknown, composite, London, Scotland, abroad and +# Scottish, Scotland but not Scottish. +SIMULATED_RECORDS = ((13, 0), (14, 0), (-1, 0), (7, 0), (11, 1), (13, 1), (11, 0)) + + +def _spi_income_tax(tmp_path, years, **kwargs): + from policyengine_uk import Microsimulation + + from policyengine_uk_data.datasets.spi import create_spi + + tab = tmp_path / "spi.tab" + gor, scot = zip(*SIMULATED_RECORDS) + _write_fake_spi(tab, gor_values=gor, maind_values=[0] * len(gor)) + _set_spi_columns( + tab, SCOT_TXP=scot, PAY=[60_000] * len(gor), AGERANGE=[3] * len(gor) + ) + sim = Microsimulation(dataset=create_spi(tab, 2022, **kwargs)) + return {year: sim.calculate("income_tax", year).values for year in years} + + +@pytest.mark.parametrize("year", [2022, 2026]) +def test_spi_income_tax_follows_scottish_taxpayer_flag(tmp_path, year): + """Equal pay, so income tax depends only on SCOT_TXP, in the data year + and after uprating. Uses the legacy SOUTH_EAST label so it runs on any + policyengine-uk release.""" + tax = _spi_income_tax(tmp_path, [year], unknown_region="SOUTH_EAST")[year] + abroad, unknown, composite, london, scotland, abroad_scot, scotland_ruk = tax + + assert london > 0 + assert abroad == unknown == composite == scotland_ruk == london + assert abroad_scot == scotland != london + + +@pytest.mark.xfail( + condition=not model_simulates_unknown_region(), + reason="policyengine-uk before 2.104.5 (PolicyEngine/policyengine-uk#1985) " + "has no rent index for Region.UNKNOWN", + raises=ParameterNotFoundError, + strict=True, +) +def test_create_spi_output_with_unknown_region_can_be_simulated(tmp_path): + """SPI records with an address abroad (13), an unknown address (14) or a + composite record (-1) keep region UNKNOWN and still run through + policyengine-uk, giving record for record the income tax of the legacy + SOUTH_EAST relabelling: the region label does not move income tax. + """ + years = [2022, 2026] + tax = _spi_income_tax(tmp_path, years) + legacy = _spi_income_tax(tmp_path, years, unknown_region="SOUTH_EAST") + + for year in years: + assert (tax[year] > 0).all() + assert (tax[year] == legacy[year]).all() diff --git a/policyengine_uk_data/utils/incomes_projection.py b/policyengine_uk_data/utils/incomes_projection.py index 144e3f833..7e0fe5e8b 100644 --- a/policyengine_uk_data/utils/incomes_projection.py +++ b/policyengine_uk_data/utils/incomes_projection.py @@ -7,6 +7,7 @@ SPI_RELEASE_NAME, SPI_TAB_FILENAME, create_spi, + model_simulates_unknown_region, ) from policyengine_uk_data.storage import STORAGE_FOLDER from policyengine_uk_data.targets.sources.hmrc_spi import ( @@ -31,6 +32,12 @@ def _read_spi_dataset_year(dataset_path) -> int: return int(store["time_period"].iloc[0]) +def _has_scottish_taxpayer_flag(dataset_path) -> bool: + # SPI H5s built before create_spi read SCOT_TXP lack this column. + with pd.HDFStore(dataset_path, mode="r") as store: + return "pays_scottish_income_tax" in store.select("person", stop=0) + + def ensure_spi_dataset() -> str: """Create the SPI H5 projection input from the current TAB release if needed. @@ -42,6 +49,7 @@ def ensure_spi_dataset() -> str: if ( dataset_path.exists() and _read_spi_dataset_year(dataset_path) == SPI_FISCAL_YEAR + and _has_scottish_taxpayer_flag(dataset_path) ): return str(dataset_path) @@ -64,9 +72,13 @@ def ensure_spi_dataset() -> str: def load_spi_dataset() -> UKSingleYearDataset: dataset = UKSingleYearDataset(ensure_spi_dataset()) - dataset.household["region"] = dataset.household["region"].replace( - {"UNKNOWN": "SOUTH_EAST"} - ) + # Older policyengine-uk releases cannot simulate an unknown region. The + # stand-in label does not move income tax, which follows the Scottish + # taxpayer flag. + if not model_simulates_unknown_region(): + dataset.household["region"] = dataset.household["region"].replace( + {"UNKNOWN": "SOUTH_EAST"} + ) return dataset From 3c37ef808fedb0c8abced657a66d2c12bcf275ff Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 5 Oct 2026 18:16:14 -0400 Subject: [PATCH 2/5] Probe the imported model for unknown-region support; tighten SPI tests Review of #540 round 1 (Sol 6.1, REQUEST CHANGES): - model_simulates_unknown_region now simulates one UNKNOWN household on the imported policyengine-uk instead of reading installed package metadata, which differs from the imported code under make data-local. Only a missing parameter for UNKNOWN reads as "cannot simulate"; any other error raises. - A differential test ties the probe to the release: where the imported model is the installed distribution, the probe agrees with version >= 2.104.5. - The unknown-region test xfails only on the missing private rent index for UNKNOWN; any other error fails it. - The region invariant test checks HMRC's codebook mapping written out in the test, not REGION_MAP itself. - New test: records whose SCOT_TXP agrees with their region keep exactly the income tax of the region-derived rule; the others change. - Income tax tests now cover 2030, the last year policyengine-uk carries dataset inputs to; the create_spi comment records that horizon. Co-Authored-By: Claude Opus 5.5 --- changelog.d/spi-unknown-region.fixed.md | 2 +- policyengine_uk_data/datasets/spi.py | 54 +++++-- policyengine_uk_data/tests/test_spi_build.py | 155 +++++++++++++++---- 3 files changed, 174 insertions(+), 37 deletions(-) diff --git a/changelog.d/spi-unknown-region.fixed.md b/changelog.d/spi-unknown-region.fixed.md index a8a4b44cf..f5111f98e 100644 --- a/changelog.d/spi-unknown-region.fixed.md +++ b/changelog.d/spi-unknown-region.fixed.md @@ -1 +1 @@ -Take Scottish taxpayer status in the SPI dataset from HMRC's `SCOT_TXP` flag rather than the region code. `GORCODE` 13, 14 and -1 (address abroad, address unknown, composite records) stay `UNKNOWN`, which policyengine-uk 2.104.5 and later can simulate; `load_spi_dataset` relabels them `SOUTH_EAST` only for older releases, and rebuilds a cached SPI H5 that predates the flag. +Take Scottish taxpayer status in the SPI dataset from HMRC's `SCOT_TXP` flag rather than the region code. `GORCODE` 13, 14 and -1 (address abroad, address unknown, composite records) stay `UNKNOWN`, which policyengine-uk 2.104.5 and later can simulate; `load_spi_dataset` relabels them `SOUTH_EAST` only when the imported model cannot simulate them, and rebuilds a cached SPI H5 that predates the flag. diff --git a/policyengine_uk_data/datasets/spi.py b/policyengine_uk_data/datasets/spi.py index a21ddb14f..9f661f8bb 100644 --- a/policyengine_uk_data/datasets/spi.py +++ b/policyengine_uk_data/datasets/spi.py @@ -1,5 +1,4 @@ -import re -from importlib.metadata import version +from functools import cache from policyengine_uk_data.storage import STORAGE_FOLDER import pandas as pd @@ -46,15 +45,49 @@ 12: "NORTHERN_IRELAND", } -# First policyengine-uk release that can simulate Region.UNKNOWN: it uprates -# their rent by the UK-wide index (PolicyEngine/policyengine-uk#1985). -UNKNOWN_REGION_MODEL_VERSION = (2, 104, 5) - +@cache def model_simulates_unknown_region() -> bool: - """Whether the installed policyengine-uk can simulate Region.UNKNOWN.""" - release = re.match(r"\d+(\.\d+)*", version("policyengine-uk")).group() - return tuple(map(int, release.split("."))) >= UNKNOWN_REGION_MODEL_VERSION + """Whether the imported policyengine-uk can simulate Region.UNKNOWN. + + Releases before 2.104.5 have no rent index for it + (PolicyEngine/policyengine-uk#1985). This simulates one household rather + than reading the installed version, which need not be the imported code + (``make data-local`` puts a checkout on PYTHONPATH). + """ + from policyengine_core.errors import ParameterNotFoundError + from policyengine_uk import Microsimulation + + ids = [1] + household = UKSingleYearDataset( + person=pd.DataFrame( + { + "person_id": ids, + "person_benunit_id": ids, + "person_household_id": ids, + "age": [40], + } + ), + benunit=pd.DataFrame({"benunit_id": ids}), + household=pd.DataFrame( + { + "household_id": ids, + "household_weight": [1.0], + "region": ["UNKNOWN"], + "rent": [0.0], + "tenure_type": ["OWNED_OUTRIGHT"], + "council_tax": [0.0], + } + ), + fiscal_year=SPI_FISCAL_YEAR, + ) + try: + Microsimulation(dataset=household) + except ParameterNotFoundError as error: + if ".UNKNOWN'" not in str(error): + raise + return False + return True def _get_marriage_allowance(fiscal_year: int) -> float: @@ -138,7 +171,8 @@ def create_spi( # disagree for some records and SCOT_TXP is also set on records with no # UK region, so SCOT_TXP decides the rates. HMRC documents it as "." or 1; # the 2022-23 tape holds 0 or 1. WELSH_TXP is not used: policyengine-uk - # has no separate Welsh rates. + # has no separate Welsh rates. policyengine-uk carries dataset inputs to + # 2030; from 2031 it derives the status from the region again. person["pays_scottish_income_tax"] = ( pd.to_numeric(df.SCOT_TXP, errors="coerce") == 1 ) diff --git a/policyengine_uk_data/tests/test_spi_build.py b/policyengine_uk_data/tests/test_spi_build.py index 14c4a5c41..a6a26109a 100644 --- a/policyengine_uk_data/tests/test_spi_build.py +++ b/policyengine_uk_data/tests/test_spi_build.py @@ -23,7 +23,6 @@ import numpy as np import pandas as pd import pytest -from policyengine_core.errors import ParameterNotFoundError if importlib.util.find_spec("policyengine_uk") is None: pytest.skip( @@ -31,6 +30,8 @@ allow_module_level=True, ) +from policyengine_core.errors import ParameterNotFoundError + from policyengine_uk_data.datasets.spi import model_simulates_unknown_region @@ -529,6 +530,24 @@ def _set_spi_columns(path, **columns): df.to_csv(path, sep="\t", index=False) +# HMRC's GORCODE codes 1-12 (SN 9422, Annex A) as policyengine-uk regions, +# written out here rather than read from REGION_MAP so a wrong mapping fails. +HMRC_REGIONS = { + 1: "NORTH_EAST", + 2: "NORTH_WEST", + 3: "YORKSHIRE", # Yorkshire and the Humber + 4: "EAST_MIDLANDS", + 5: "WEST_MIDLANDS", + 6: "EAST_OF_ENGLAND", + 7: "LONDON", + 8: "SOUTH_EAST", + 9: "SOUTH_WEST", + 10: "WALES", + 11: "SCOTLAND", + 12: "NORTHERN_IRELAND", +} + + @pytest.mark.parametrize("not_scottish", [0, ".", ""]) def test_create_spi_region_and_scottish_taxpayer_invariants(tmp_path, not_scottish): """For every GORCODE (documented 1-14, composite -1, undocumented 99) and @@ -540,7 +559,7 @@ def test_create_spi_region_and_scottish_taxpayer_invariants(tmp_path, not_scotti from policyengine_uk.variables.household.demographic.geography import Region - from policyengine_uk_data.datasets.spi import REGION_MAP, create_spi + from policyengine_uk_data.datasets.spi import create_spi cases = list(product([-1, *range(1, 15), 99], (not_scottish, 1))) gor = [g for g, _ in cases] @@ -552,7 +571,7 @@ def test_create_spi_region_and_scottish_taxpayer_invariants(tmp_path, not_scotti ds = create_spi(tab, 2022) regions = ds.household["region"].tolist() - assert regions == [REGION_MAP.get(g, "UNKNOWN") for g in gor] + assert regions == [HMRC_REGIONS.get(g, "UNKNOWN") for g in gor] assert set(regions) <= {region.name for region in Region} assert ds.person["pays_scottish_income_tax"].tolist() == [s == 1 for s in scot] # Address abroad, address unknown and composite records stay UNKNOWN even @@ -577,12 +596,74 @@ def test_create_spi_scottish_taxpayer_status_survives_h5_round_trip(tmp_path): assert loaded.household["region"].tolist() == ["UNKNOWN", "SCOTLAND", "LONDON"] +UNKNOWN_RENT_INDEX = ( + "gov.economic_assumptions.yoy_growth.ons.private_rental_prices.UNKNOWN" +) + + +@pytest.mark.parametrize( + "missing, expected", [(None, True), (UNKNOWN_RENT_INDEX, False)] +) +def test_unknown_region_probe_reads_the_simulation(monkeypatch, missing, expected): + import policyengine_uk + + def simulate(dataset): + assert dataset.household["region"].tolist() == ["UNKNOWN"] + if missing: + raise ParameterNotFoundError(missing, "2023-01-01") + + monkeypatch.setattr(policyengine_uk, "Microsimulation", simulate) + + assert model_simulates_unknown_region.__wrapped__() is expected + + +def test_unknown_region_probe_raises_other_missing_parameters(monkeypatch): + import policyengine_uk + + def simulate(dataset): + raise ParameterNotFoundError("gov.hmrc.income_tax.rates.uk", "2023-01-01") + + monkeypatch.setattr(policyengine_uk, "Microsimulation", simulate) + + with pytest.raises(ParameterNotFoundError, match=r"rates\.uk'"): + model_simulates_unknown_region.__wrapped__() + + +def test_unknown_region_probe_agrees_with_policyengine_uk_release(): + """2.104.5 is the first policyengine-uk release with + PolicyEngine/policyengine-uk#1985. Where the imported model is the + installed release, the probe agrees with its version.""" + from importlib.metadata import PackageNotFoundError, distribution + from pathlib import Path + + import policyengine_uk + from packaging.version import Version + + try: + release = distribution("policyengine-uk") + except PackageNotFoundError: + pytest.skip("policyengine-uk is not installed as a distribution") + installed = Path(release.locate_file("policyengine_uk/__init__.py")) + if not installed.exists() or not installed.samefile(policyengine_uk.__file__): + pytest.skip("the imported policyengine-uk is not the installed release") + + assert model_simulates_unknown_region() == ( + Version(release.version) >= Version("2.104.5") + ) + + # GORCODE, SCOT_TXP: abroad, unknown, composite, London, Scotland, abroad and # Scottish, Scotland but not Scottish. SIMULATED_RECORDS = ((13, 0), (14, 0), (-1, 0), (7, 0), (11, 1), (13, 1), (11, 0)) +# The data year, an uprated year and 2030, the last year policyengine-uk +# carries dataset inputs to. +YEARS = [2022, 2026, 2030] -def _spi_income_tax(tmp_path, years, **kwargs): +def _spi_income_tax(tmp_path, region_rule=False, **kwargs): + """Income tax on SIMULATED_RECORDS, each paid £60,000, in YEARS. With + region_rule, the Scottish taxpayer flag is dropped, so policyengine-uk + derives it from the region as it did before create_spi read SCOT_TXP.""" from policyengine_uk import Microsimulation from policyengine_uk_data.datasets.spi import create_spi @@ -593,40 +674,62 @@ def _spi_income_tax(tmp_path, years, **kwargs): _set_spi_columns( tab, SCOT_TXP=scot, PAY=[60_000] * len(gor), AGERANGE=[3] * len(gor) ) - sim = Microsimulation(dataset=create_spi(tab, 2022, **kwargs)) - return {year: sim.calculate("income_tax", year).values for year in years} + dataset = create_spi(tab, 2022, **kwargs) + if region_rule: + dataset.person = dataset.person.drop(columns="pays_scottish_income_tax") + sim = Microsimulation(dataset=dataset) + return {year: sim.calculate("income_tax", year).values for year in YEARS} -@pytest.mark.parametrize("year", [2022, 2026]) -def test_spi_income_tax_follows_scottish_taxpayer_flag(tmp_path, year): +def test_spi_income_tax_follows_scottish_taxpayer_flag(tmp_path): """Equal pay, so income tax depends only on SCOT_TXP, in the data year and after uprating. Uses the legacy SOUTH_EAST label so it runs on any policyengine-uk release.""" - tax = _spi_income_tax(tmp_path, [year], unknown_region="SOUTH_EAST")[year] - abroad, unknown, composite, london, scotland, abroad_scot, scotland_ruk = tax + for year, tax in _spi_income_tax(tmp_path, unknown_region="SOUTH_EAST").items(): + abroad, unknown, composite, london, scotland, abroad_scot, scotland_ruk = tax - assert london > 0 - assert abroad == unknown == composite == scotland_ruk == london - assert abroad_scot == scotland != london + assert london > 0, year + assert abroad == unknown == composite == scotland_ruk == london, year + assert abroad_scot == scotland != london, year + + +def test_spi_income_tax_changes_only_where_scottish_flag_and_region_disagree( + tmp_path, +): + """Records whose SCOT_TXP agrees with their region get exactly the income + tax they got when the region decided Scottish status. The two records + where they disagree change.""" + label = "UNKNOWN" if model_simulates_unknown_region() else "SOUTH_EAST" + flag = _spi_income_tax(tmp_path, unknown_region=label) + region = _spi_income_tax(tmp_path, region_rule=True, unknown_region=label) + agrees = np.array([(g == 11) == (s == 1) for g, s in SIMULATED_RECORDS]) + + for year in YEARS: + assert (flag[year][agrees] == region[year][agrees]).all(), year + assert (flag[year][~agrees] != region[year][~agrees]).all(), year -@pytest.mark.xfail( - condition=not model_simulates_unknown_region(), - reason="policyengine-uk before 2.104.5 (PolicyEngine/policyengine-uk#1985) " - "has no rent index for Region.UNKNOWN", - raises=ParameterNotFoundError, - strict=True, -) def test_create_spi_output_with_unknown_region_can_be_simulated(tmp_path): """SPI records with an address abroad (13), an unknown address (14) or a composite record (-1) keep region UNKNOWN and still run through policyengine-uk, giving record for record the income tax of the legacy SOUTH_EAST relabelling: the region label does not move income tax. + Models without PolicyEngine/policyengine-uk#1985 must fail on the missing + rent index, and on nothing else. """ - years = [2022, 2026] - tax = _spi_income_tax(tmp_path, years) - legacy = _spi_income_tax(tmp_path, years, unknown_region="SOUTH_EAST") + if not model_simulates_unknown_region(): + with pytest.raises( + ParameterNotFoundError, match=r"private_rental_prices\.UNKNOWN'" + ): + _spi_income_tax(tmp_path) + pytest.xfail( + "policyengine-uk before 2.104.5 (PolicyEngine/policyengine-uk#1985) " + "has no rent index for Region.UNKNOWN" + ) + + tax = _spi_income_tax(tmp_path) + legacy = _spi_income_tax(tmp_path, unknown_region="SOUTH_EAST") - for year in years: - assert (tax[year] > 0).all() - assert (tax[year] == legacy[year]).all() + for year in YEARS: + assert (tax[year] > 0).all(), year + assert (tax[year] == legacy[year]).all(), year From c65d3810ced2f1ca31cab7f6b63e82b45acb5289 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 5 Oct 2026 20:04:36 -0400 Subject: [PATCH 3/5] Decide unknown-region support with a known-region control Review of #540 round 2 (Sol 6.1, REQUEST CHANGES): - model_simulates_unknown_region no longer matches error text. A failure with region UNKNOWN reads as "cannot simulate" only if the same household labelled SOUTH_EAST simulates, which is exactly when the loader's relabel helps; otherwise the control's error is raised. The household mirrors the SPI-shaped fixture policyengine-uk's own test_rent_uprating.py simulates. (ParameterNotFoundError.name is reset to None by AttributeError.__init__, so the parameter name is not available to match.) - Probe tests cover unrelated parameters ending in .UNKNOWN, unrelated parameters and schema errors (all raised), and a non-parameter failure that relabelling cures (read as "no"). - The release differential test runs only for an unmodified final release installed from a package index and imported from there: it skips pre, dev and local versions, direct-URL or editable installs, and an economic_assumptions.py that does not match the release's RECORD. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk_data/datasets/spi.py | 42 ++++++----- policyengine_uk_data/tests/test_spi_build.py | 77 +++++++++++++++----- 2 files changed, 83 insertions(+), 36 deletions(-) diff --git a/policyengine_uk_data/datasets/spi.py b/policyengine_uk_data/datasets/spi.py index 9f661f8bb..9108d6af5 100644 --- a/policyengine_uk_data/datasets/spi.py +++ b/policyengine_uk_data/datasets/spi.py @@ -46,26 +46,18 @@ } -@cache -def model_simulates_unknown_region() -> bool: - """Whether the imported policyengine-uk can simulate Region.UNKNOWN. - - Releases before 2.104.5 have no rent index for it - (PolicyEngine/policyengine-uk#1985). This simulates one household rather - than reading the installed version, which need not be the imported code - (``make data-local`` puts a checkout on PYTHONPATH). - """ - from policyengine_core.errors import ParameterNotFoundError - from policyengine_uk import Microsimulation - +def _spi_shaped_household(region: str) -> UKSingleYearDataset: + # The fixture policyengine-uk's own test_rent_uprating.py simulates with + # Region.UNKNOWN, so the model keeps supporting this shape. ids = [1] - household = UKSingleYearDataset( + return UKSingleYearDataset( person=pd.DataFrame( { "person_id": ids, "person_benunit_id": ids, "person_household_id": ids, "age": [40], + "employment_income": [60_000.0], } ), benunit=pd.DataFrame({"benunit_id": ids}), @@ -73,7 +65,7 @@ def model_simulates_unknown_region() -> bool: { "household_id": ids, "household_weight": [1.0], - "region": ["UNKNOWN"], + "region": [region], "rent": [0.0], "tenure_type": ["OWNED_OUTRIGHT"], "council_tax": [0.0], @@ -81,11 +73,25 @@ def model_simulates_unknown_region() -> bool: ), fiscal_year=SPI_FISCAL_YEAR, ) + + +@cache +def model_simulates_unknown_region() -> bool: + """Whether the imported policyengine-uk can simulate Region.UNKNOWN. + + Releases before 2.104.5 have no rent index for it + (PolicyEngine/policyengine-uk#1985). This simulates one household rather + than reading the installed version, which need not be the imported code + (``make data-local`` puts a checkout on PYTHONPATH). A failure counts as + "no" only if the same household labelled SOUTH_EAST simulates; otherwise + that error is raised, since relabelling would not help. + """ + from policyengine_uk import Microsimulation + try: - Microsimulation(dataset=household) - except ParameterNotFoundError as error: - if ".UNKNOWN'" not in str(error): - raise + Microsimulation(dataset=_spi_shaped_household("UNKNOWN")) + except Exception: + Microsimulation(dataset=_spi_shaped_household("SOUTH_EAST")) return False return True diff --git a/policyengine_uk_data/tests/test_spi_build.py b/policyengine_uk_data/tests/test_spi_build.py index a6a26109a..970170f66 100644 --- a/policyengine_uk_data/tests/test_spi_build.py +++ b/policyengine_uk_data/tests/test_spi_build.py @@ -596,60 +596,101 @@ def test_create_spi_scottish_taxpayer_status_survives_h5_round_trip(tmp_path): assert loaded.household["region"].tolist() == ["UNKNOWN", "SCOTLAND", "LONDON"] -UNKNOWN_RENT_INDEX = ( +def _missing(name): + return ParameterNotFoundError(name, "2023-01-01", "rent") + + +UNKNOWN_RENT_INDEX = _missing( "gov.economic_assumptions.yoy_growth.ons.private_rental_prices.UNKNOWN" ) @pytest.mark.parametrize( - "missing, expected", [(None, True), (UNKNOWN_RENT_INDEX, False)] + "errors, expected", + [ + ({}, True), + ({"UNKNOWN": UNKNOWN_RENT_INDEX}, False), + # Any failure that relabelling to SOUTH_EAST cures. + ({"UNKNOWN": KeyError("UNKNOWN")}, False), + ], ) -def test_unknown_region_probe_reads_the_simulation(monkeypatch, missing, expected): +def test_unknown_region_probe_reads_the_simulation(monkeypatch, errors, expected): import policyengine_uk + regions = [] + def simulate(dataset): - assert dataset.household["region"].tolist() == ["UNKNOWN"] - if missing: - raise ParameterNotFoundError(missing, "2023-01-01") + (region,) = dataset.household["region"] + regions.append(region) + if region in errors: + raise errors[region] monkeypatch.setattr(policyengine_uk, "Microsimulation", simulate) assert model_simulates_unknown_region.__wrapped__() is expected + assert regions == (["UNKNOWN"] if expected else ["UNKNOWN", "SOUTH_EAST"]) -def test_unknown_region_probe_raises_other_missing_parameters(monkeypatch): +@pytest.mark.parametrize( + "error", + [ + _missing("gov.hmrc.unrelated.UNKNOWN"), + _missing("gov.hmrc.income_tax.rates.uk"), + KeyError("employment_income"), + ], +) +def test_unknown_region_probe_raises_failures_relabelling_would_not_cure( + monkeypatch, error +): + """A failure that the SOUTH_EAST household shares is raised, not read as + a missing UNKNOWN index.""" import policyengine_uk def simulate(dataset): - raise ParameterNotFoundError("gov.hmrc.income_tax.rates.uk", "2023-01-01") + raise error monkeypatch.setattr(policyengine_uk, "Microsimulation", simulate) - with pytest.raises(ParameterNotFoundError, match=r"rates\.uk'"): + with pytest.raises(type(error)) as raised: model_simulates_unknown_region.__wrapped__() + assert raised.value is error def test_unknown_region_probe_agrees_with_policyengine_uk_release(): """2.104.5 is the first policyengine-uk release with - PolicyEngine/policyengine-uk#1985. Where the imported model is the - installed release, the probe agrees with its version.""" + PolicyEngine/policyengine-uk#1985. For an unmodified final release from a + package index that is also the imported code, the probe agrees with the + release number.""" + import base64 + import hashlib from importlib.metadata import PackageNotFoundError, distribution from pathlib import Path - import policyengine_uk from packaging.version import Version + from policyengine_uk.data import economic_assumptions try: release = distribution("policyengine-uk") except PackageNotFoundError: pytest.skip("policyengine-uk is not installed as a distribution") - installed = Path(release.locate_file("policyengine_uk/__init__.py")) - if not installed.exists() or not installed.samefile(policyengine_uk.__file__): + version = Version(release.version) + if version.is_prerelease or version.is_devrelease or version.local: + pytest.skip(f"policyengine-uk {version} is not a final release") + if release.read_text("direct_url.json") is not None: + pytest.skip("policyengine-uk was not installed from a package index") + # #1985 changed this module, so it must be the installed file, unmodified. + path = "policyengine_uk/data/economic_assumptions.py" + installed = Path(release.locate_file(path)) + if not installed.exists() or not installed.samefile(economic_assumptions.__file__): pytest.skip("the imported policyengine-uk is not the installed release") - - assert model_simulates_unknown_region() == ( - Version(release.version) >= Version("2.104.5") - ) + record = {file.as_posix(): file.hash for file in release.files or []}.get(path) + if record is None: + pytest.skip("policyengine-uk's RECORD does not list the module") + digest = hashlib.new(record.mode, installed.read_bytes()).digest() + if base64.urlsafe_b64encode(digest).rstrip(b"=").decode() != record.value: + pytest.skip("the installed policyengine-uk has been modified") + + assert model_simulates_unknown_region() == (version >= Version("2.104.5")) # GORCODE, SCOT_TXP: abroad, unknown, composite, London, Scotland, abroad and From 4f86ceeeb844d6fcd76f8da7e68e5d2a8062525c Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Mon, 5 Oct 2026 20:57:21 -0400 Subject: [PATCH 4/5] Scope the probe and release-test wording; pin which error the probe raises Review of #540 round 3 (Sol 6.1, APPROVE WITH CHANGES, three nits): - The probe docstring says what it establishes: that the imported model can build a simulation of an SPI-shaped household in Region.UNKNOWN. - The release differential test describes its checks as they are: a final release not installed from a direct URL, whose imported economic_assumptions.py is the installed file and matches its RECORD. - The shared-failure test gives the UNKNOWN and SOUTH_EAST households distinct errors and requires the control's, with the UNKNOWN error as its context. A mutant that re-raised the UNKNOWN error had survived. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk_data/datasets/spi.py | 9 +++++---- policyengine_uk_data/tests/test_spi_build.py | 20 ++++++++++++-------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/policyengine_uk_data/datasets/spi.py b/policyengine_uk_data/datasets/spi.py index 9108d6af5..b71eccaab 100644 --- a/policyengine_uk_data/datasets/spi.py +++ b/policyengine_uk_data/datasets/spi.py @@ -77,13 +77,14 @@ def _spi_shaped_household(region: str) -> UKSingleYearDataset: @cache def model_simulates_unknown_region() -> bool: - """Whether the imported policyengine-uk can simulate Region.UNKNOWN. + """Whether the imported policyengine-uk can build a simulation of an + SPI-shaped household in Region.UNKNOWN. Releases before 2.104.5 have no rent index for it - (PolicyEngine/policyengine-uk#1985). This simulates one household rather - than reading the installed version, which need not be the imported code + (PolicyEngine/policyengine-uk#1985). This builds one household rather than + reading the installed version, which need not be the imported code (``make data-local`` puts a checkout on PYTHONPATH). A failure counts as - "no" only if the same household labelled SOUTH_EAST simulates; otherwise + "no" only if the same household labelled SOUTH_EAST builds; otherwise that error is raised, since relabelling would not help. """ from policyengine_uk import Microsimulation diff --git a/policyengine_uk_data/tests/test_spi_build.py b/policyengine_uk_data/tests/test_spi_build.py index 970170f66..592c1daba 100644 --- a/policyengine_uk_data/tests/test_spi_build.py +++ b/policyengine_uk_data/tests/test_spi_build.py @@ -642,25 +642,29 @@ def simulate(dataset): def test_unknown_region_probe_raises_failures_relabelling_would_not_cure( monkeypatch, error ): - """A failure that the SOUTH_EAST household shares is raised, not read as - a missing UNKNOWN index.""" + """When the SOUTH_EAST household fails too, its error is raised, not read + as a missing UNKNOWN index.""" import policyengine_uk + original = RuntimeError("the UNKNOWN household failed") + def simulate(dataset): - raise error + (region,) = dataset.household["region"] + raise original if region == "UNKNOWN" else error monkeypatch.setattr(policyengine_uk, "Microsimulation", simulate) with pytest.raises(type(error)) as raised: model_simulates_unknown_region.__wrapped__() assert raised.value is error + assert raised.value.__context__ is original def test_unknown_region_probe_agrees_with_policyengine_uk_release(): """2.104.5 is the first policyengine-uk release with - PolicyEngine/policyengine-uk#1985. For an unmodified final release from a - package index that is also the imported code, the probe agrees with the - release number.""" + PolicyEngine/policyengine-uk#1985. For a final release not installed from + a direct URL, whose imported economic_assumptions.py is the installed file + and matches its RECORD, the probe agrees with the release number.""" import base64 import hashlib from importlib.metadata import PackageNotFoundError, distribution @@ -677,7 +681,7 @@ def test_unknown_region_probe_agrees_with_policyengine_uk_release(): if version.is_prerelease or version.is_devrelease or version.local: pytest.skip(f"policyengine-uk {version} is not a final release") if release.read_text("direct_url.json") is not None: - pytest.skip("policyengine-uk was not installed from a package index") + pytest.skip("policyengine-uk was installed from a direct URL") # #1985 changed this module, so it must be the installed file, unmodified. path = "policyengine_uk/data/economic_assumptions.py" installed = Path(release.locate_file(path)) @@ -688,7 +692,7 @@ def test_unknown_region_probe_agrees_with_policyengine_uk_release(): pytest.skip("policyengine-uk's RECORD does not list the module") digest = hashlib.new(record.mode, installed.read_bytes()).digest() if base64.urlsafe_b64encode(digest).rstrip(b"=").decode() != record.value: - pytest.skip("the installed policyengine-uk has been modified") + pytest.skip("the installed economic_assumptions.py has been modified") assert model_simulates_unknown_region() == (version >= Version("2.104.5")) From 4fbd8be43b7dc550d9f6c2871ddc7270fec38c01 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 7 Oct 2026 14:40:42 -0400 Subject: [PATCH 5/5] Align lockfile project version with batch release 1.58.0 --- uv.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uv.lock b/uv.lock index 5ae821352..dce9c8285 100644 --- a/uv.lock +++ b/uv.lock @@ -1433,7 +1433,7 @@ wheels = [ [[package]] name = "policyengine-uk-data" -version = "1.57.4" +version = "1.58.0" source = { editable = "." } dependencies = [ { name = "google-auth" },