From 4e6d10f1ac4b891215a1f31c08dc88e57afd45ef Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Wed, 28 Oct 2026 22:06:58 -0400 Subject: [PATCH 1/4] Set income_not_otherwise_charged to FRS odd-job income policyengine-uk's trading allowance relieves income charged under ITTOIA 2005 Part 5 Chapter 8 (s. 783AB) and reads it from income_not_otherwise_charged (PolicyEngine/policyengine-uk#2726). Of the fields the build sums into miscellaneous_income, only the FRS odd-job amount (ODDJOB.OJAMT) records casual income that Chapter 8 charges, so the build now returns it alongside miscellaneous_income. The SPI-synthetic copy keeps none: its trading profits are SPI PROFITS, which may already have used the allowance. miscellaneous_income itself stays bitwise unchanged whenever no odd-job amount carries an FRS missing-value code; a code now counts as zero. Co-Authored-By: Claude Opus 5.5 --- .../income-not-otherwise-charged.added.md | 1 + policyengine_uk_data/datasets/frs.py | 77 ++++-- .../datasets/imputations/income.py | 21 ++ .../test_income_not_otherwise_charged.py | 258 ++++++++++++++++++ .../tests/test_non_negative_incomes.py | 1 + 5 files changed, 342 insertions(+), 16 deletions(-) create mode 100644 changelog.d/income-not-otherwise-charged.added.md create mode 100644 policyengine_uk_data/tests/test_income_not_otherwise_charged.py diff --git a/changelog.d/income-not-otherwise-charged.added.md b/changelog.d/income-not-otherwise-charged.added.md new file mode 100644 index 000000000..ad95d1c79 --- /dev/null +++ b/changelog.d/income-not-otherwise-charged.added.md @@ -0,0 +1 @@ +Set `income_not_otherwise_charged` to FRS odd-job income (ITTOIA 2005 Part 5 Chapter 8), so policyengine-uk's trading allowance relieves it. The other parts of `miscellaneous_income` (royalties, a sleeping partner's share, overseas pensions, a child's spare-time earnings, trust income and an organisation's allowance) stay outside it, and the SPI-synthetic copy holds none. diff --git a/policyengine_uk_data/datasets/frs.py b/policyengine_uk_data/datasets/frs.py index ddd002606..e82607c28 100644 --- a/policyengine_uk_data/datasets/frs.py +++ b/policyengine_uk_data/datasets/frs.py @@ -352,6 +352,63 @@ def frs_property_income(person: pd.DataFrame, household: pd.DataFrame) -> np.nda ).values +MISC_INCOME_FIELDS = [ + "allpay2", + "royyr2", + "royyr3", + "royyr4", + "chamtern", + "chamttst", +] + + +def frs_miscellaneous_income( + person: pd.DataFrame, oddjob: pd.DataFrame +) -> tuple[np.ndarray, np.ndarray]: + """Annual miscellaneous income and the income not otherwise charged in it. + + Returns ``(miscellaneous_income, income_not_otherwise_charged)``, aligned + with ``person.person_id``. The second is part of the first, so it never + exceeds it. + + Miscellaneous income adds odd-job income to six amounts on the person + tables, whose sum is floored at zero: ALLPAY2 (allowance from an + organisation), ROYYR2 (royalties), ROYYR3 (income as a sleeping + partner), ROYYR4 (overseas pension), and from the child table CHAMTERN + (earnings from a spare time job) and CHAMTTST (income from a trust). + + Odd-job income comes from the ODDJOB table. The FRS asks (OddJob) about + money the person received in the last 4 weeks from babysitting, acting as + a mail order agent, or an odd job, occasional work or professional advice + (ODDTYPE 1 to 3), that the employment and self-employment questions have + not covered. A person does an odd job once, without regularity or + continuity. OJAMT is the amount the person received and OJNOW whether + they did the work in the last 7 days. Jobs from the last 7 days count, as + weekly amounts (#640). Negative OJAMT values are FRS missing-value codes + (-1 to -9), not amounts, so they count as zero. + + Odd-job income is the income not otherwise charged: casual income that + no other provision charges, which ITTOIA 2005 Part 5 Chapter 8 charges + (s. 687) and the trading allowance relieves (s. 783AB). Casual work + that amounts to a trade takes the same allowance. The six other amounts + are not Chapter 8 income: royalties fall under Part 5 Chapter 2, a + sleeping partner's share is partnership trading income, an overseas + pension is pension income (ITEPA 2003 Part 9), a child's spare time job + is a job, trust income is an annual payment (Part 5 Chapter 7) or keeps + its source, and the organisation's allowance is a transfer, which + private_transfer_income also counts (#627). + """ + odd_job_income = sum_to_entity( + oddjob.ojamt.clip(lower=0) * (oddjob.ojnow == 1), + oddjob.person_id, + person.person_id, + ) + miscellaneous_income = ( + odd_job_income + sum_from_positive_fields(person, MISC_INCOME_FIELDS) + ) * WEEKS_IN_YEAR + return miscellaneous_income, odd_job_income * WEEKS_IN_YEAR + + def derive_is_in_non_advanced_education( current_education, is_apprentice=None, @@ -1376,22 +1433,10 @@ def determine_education_level(fted_val, typeed2_val, age_val): * WEEKS_IN_YEAR ) - odd_job_income = sum_to_entity( - oddjob.ojamt * (oddjob.ojnow == 1), oddjob.person_id, person.person_id - ) - - MISC_INCOME_FIELDS = [ - "allpay2", - "royyr2", - "royyr3", - "royyr4", - "chamtern", - "chamttst", - ] - - pe_person["miscellaneous_income"] = ( - odd_job_income + sum_from_positive_fields(person, MISC_INCOME_FIELDS) - ) * WEEKS_IN_YEAR + ( + pe_person["miscellaneous_income"], + pe_person["income_not_otherwise_charged"], + ) = frs_miscellaneous_income(person, oddjob) PRIVATE_TRANSFER_INCOME_FIELDS = [ "apamt", diff --git a/policyengine_uk_data/datasets/imputations/income.py b/policyengine_uk_data/datasets/imputations/income.py index 799b5f35e..47bf712f6 100644 --- a/policyengine_uk_data/datasets/imputations/income.py +++ b/policyengine_uk_data/datasets/imputations/income.py @@ -474,6 +474,26 @@ def clear_frs_reported_capital(dataset: UKSingleYearDataset) -> UKSingleYearData return dataset +def clear_income_not_otherwise_charged( + dataset: UKSingleYearDataset, +) -> UKSingleYearDataset: + """Set ``income_not_otherwise_charged`` to zero. + + The build applies it to the SPI-synthetic copy. Its trading profits are + SPI PROFITS, which ``datasets/spi.py`` takes to be after the trading + allowance, while its miscellaneous income stays its FRS donor's (neither + imputation stage replaces it). policyengine-uk gives a person's trades the + allowance first and their income not otherwise charged what is left, so + on these rows the donor's odd-job income would take an allowance the SPI + profit may already have used. At zero, policyengine-uk taxes the copy's + miscellaneous income in full, as it did before it applied the allowance + to that income. + """ + if "income_not_otherwise_charged" in dataset.person.columns: + dataset.person["income_not_otherwise_charged"] = 0.0 + return dataset + + def impute_income(dataset: UKSingleYearDataset) -> UKSingleYearDataset: """ Impute detailed income components using trained model. @@ -503,6 +523,7 @@ def impute_income(dataset: UKSingleYearDataset) -> UKSingleYearDataset: zero_weight_copy.household.household_weight = 0 zero_weight_copy.household["household_is_spi_synthetic"] = True zero_weight_copy = clear_frs_reported_capital(zero_weight_copy) + zero_weight_copy = clear_income_not_otherwise_charged(zero_weight_copy) zero_weight_copy = subsample_dataset(zero_weight_copy, 10_000) model = create_income_model() diff --git a/policyengine_uk_data/tests/test_income_not_otherwise_charged.py b/policyengine_uk_data/tests/test_income_not_otherwise_charged.py new file mode 100644 index 000000000..ed4507092 --- /dev/null +++ b/policyengine_uk_data/tests/test_income_not_otherwise_charged.py @@ -0,0 +1,258 @@ +"""FRS odd-job income is the income not otherwise charged. + +policyengine-uk's trading allowance relieves income chargeable under ITTOIA +2005 Part 5 Chapter 8 (s. 783AB), which it reads from +``income_not_otherwise_charged``, the part of ``miscellaneous_income`` that +Chapter 8 charges. The build sets it to the FRS odd-job income in +``miscellaneous_income``. + +Invariants, for any person table and ODDJOB table (amounts including FRS +missing-value codes and blanks): + +1. Both outputs are non-negative, and the income not otherwise charged never + exceeds miscellaneous income. +2. Conservation: miscellaneous income less the income not otherwise charged + is the annualised floored sum of the six other fields, and the income not + otherwise charged is the annualised odd-job income from jobs in the last + 7 days. +3. Raising an odd-job amount never lowers either output; the six other fields + never change the income not otherwise charged. +4. Differential: miscellaneous income is what the build computed before this + change whenever no odd-job amount is a missing-value code. + +And for the built datasets: + +5. The FRS and the enhanced FRS keep invariant 1 on every row. +6. SPI-donor rows of the enhanced FRS hold none (their trading profits are + SPI PROFITS, which may already have used the allowance). +7. The uprating table moves it exactly as miscellaneous income, so the bound + holds in every year, once policyengine-uk reads it. + +The data checks assert on counts only, so a failure never prints a record's +amounts (the FRS is licensed microdata). +""" + +import numpy as np +import pandas as pd +import pytest +from hypothesis import assume, given, settings +from hypothesis import strategies as st +from policyengine_uk.data import UKSingleYearDataset + +from policyengine_uk_data.datasets.frs import ( + MISC_INCOME_FIELDS, + WEEKS_IN_YEAR, + frs_miscellaneous_income, +) +from policyengine_uk_data.datasets.imputations.income import ( + clear_income_not_otherwise_charged, +) +from policyengine_uk_data.storage import STORAGE_FOLDER +from policyengine_uk_data.utils.datasets import ( + sum_from_positive_fields, + sum_to_entity, +) + +VARIABLE = "income_not_otherwise_charged" +MISSING_CODES = st.sampled_from([-9.0, -8.0, -7.0, -1.0]) +weekly_amounts = st.one_of( + st.floats(0, 5_000, allow_nan=False, allow_infinity=False), + st.just(0.0), + MISSING_CODES, +) + + +@st.composite +def frs_tables(draw, missing_odd_job_amounts=True): + """A person table with the six fields and an ODDJOB table for it.""" + n_people = draw(st.integers(1, 8)) + person_ids = np.arange(n_people) * 7 + 1001 + person = pd.DataFrame({"person_id": person_ids}) + for field in MISC_INCOME_FIELDS: + person[field] = draw( + st.lists(weekly_amounts, min_size=n_people, max_size=n_people) + ) + n_jobs = draw(st.integers(0, 12)) + amounts = st.one_of( + st.floats(0, 5_000, allow_nan=False, allow_infinity=False), + st.just(np.nan), + *([MISSING_CODES] if missing_odd_job_amounts else []), + ) + oddjob = pd.DataFrame( + { + "person_id": draw( + st.lists( + st.sampled_from(list(person_ids)), + min_size=n_jobs, + max_size=n_jobs, + ) + ), + "ojamt": draw(st.lists(amounts, min_size=n_jobs, max_size=n_jobs)), + "ojnow": draw( + st.lists( + st.sampled_from([1.0, 2.0, -1.0, np.nan]), + min_size=n_jobs, + max_size=n_jobs, + ) + ), + }, + dtype=float, + ) + return person, oddjob + + +@settings(max_examples=300, deadline=None, derandomize=True) +@given(frs_tables()) +def test_non_negative_and_within_miscellaneous_income(tables): + person, oddjob = tables + miscellaneous, chapter_8 = frs_miscellaneous_income(person, oddjob) + assert np.all(chapter_8 >= 0) + assert np.all(miscellaneous >= 0) + assert np.all(chapter_8 <= miscellaneous) + + +@settings(max_examples=300, deadline=None, derandomize=True) +@given(frs_tables()) +def test_conserves_odd_job_and_other_income(tables): + person, oddjob = tables + miscellaneous, chapter_8 = frs_miscellaneous_income(person, oddjob) + rest = sum_from_positive_fields(person, MISC_INCOME_FIELDS) * WEEKS_IN_YEAR + np.testing.assert_allclose(miscellaneous - chapter_8, rest, rtol=1e-12, atol=1e-6) + done_last_week = oddjob.ojnow == 1 + expected = [ + oddjob.ojamt[done_last_week & (oddjob.person_id == pid)].clip(lower=0).sum() + * WEEKS_IN_YEAR + for pid in person.person_id + ] + np.testing.assert_allclose(chapter_8, expected, rtol=1e-12, atol=1e-6) + + +@settings(max_examples=300, deadline=None, derandomize=True) +@given( + frs_tables(), + st.integers(0, 11), + st.floats(0, 5_000, allow_nan=False), + st.floats(-5_000, 5_000, allow_nan=False), +) +def test_monotone_in_odd_jobs_and_blind_to_other_fields( + tables, job, rise, other_change +): + person, oddjob = tables + miscellaneous, chapter_8 = frs_miscellaneous_income(person, oddjob) + + if job < len(oddjob): + raised = oddjob.copy() + amount = raised.loc[job, "ojamt"] + raised.loc[job, "ojamt"] = (0 if np.isnan(amount) else max(amount, 0)) + rise + higher_miscellaneous, higher_chapter_8 = frs_miscellaneous_income( + person, raised + ) + assert np.all(higher_chapter_8 >= chapter_8) + assert np.all(higher_miscellaneous >= miscellaneous - 1e-6) + + changed = person.copy() + changed[MISC_INCOME_FIELDS[0]] += other_change + _, unchanged_chapter_8 = frs_miscellaneous_income(changed, oddjob) + np.testing.assert_array_equal(unchanged_chapter_8, chapter_8) + + +def _miscellaneous_income_before_this_change(person, oddjob): + """The build's miscellaneous income at policyengine-uk-data 1.59.1.""" + odd_job_income = sum_to_entity( + oddjob.ojamt * (oddjob.ojnow == 1), oddjob.person_id, person.person_id + ) + return ( + odd_job_income + sum_from_positive_fields(person, MISC_INCOME_FIELDS) + ) * WEEKS_IN_YEAR + + +@settings(max_examples=300, deadline=None, derandomize=True) +@given(frs_tables(missing_odd_job_amounts=False)) +def test_miscellaneous_income_is_unchanged(tables): + person, oddjob = tables + assume((oddjob.ojamt.fillna(0) >= 0).all()) + miscellaneous, _ = frs_miscellaneous_income(person, oddjob) + np.testing.assert_array_equal( + miscellaneous, _miscellaneous_income_before_this_change(person, oddjob) + ) + + +def _unit_dataset(chapter_8=None) -> UKSingleYearDataset: + person = pd.DataFrame( + { + "person_id": [1, 2], + "person_benunit_id": [1, 1], + "person_household_id": [1, 1], + "miscellaneous_income": [900.0, 50.0], + } + ) + if chapter_8 is not None: + person[VARIABLE] = chapter_8 + return UKSingleYearDataset( + person=person, + benunit=pd.DataFrame({"benunit_id": [1]}), + household=pd.DataFrame({"household_id": [1]}), + fiscal_year=2024, + ) + + +def test_clear_zeroes_only_the_income_not_otherwise_charged(): + cleared = clear_income_not_otherwise_charged(_unit_dataset([800.0, 50.0])) + assert cleared.person[VARIABLE].tolist() == [0.0, 0.0] + assert cleared.person.miscellaneous_income.tolist() == [900.0, 50.0] + + +def test_clear_adds_no_column_to_a_dataset_without_one(): + cleared = clear_income_not_otherwise_charged(_unit_dataset()) + assert VARIABLE not in cleared.person.columns + + +def _bound_violations(person, mask=None) -> dict: + chapter_8 = person[VARIABLE].to_numpy() + miscellaneous = person["miscellaneous_income"].to_numpy() + keep = np.ones(len(person), dtype=bool) if mask is None else mask + return { + "negative": int((chapter_8[keep] < 0).sum()), + "above_miscellaneous_income": int((chapter_8 > miscellaneous)[keep].sum()), + "not_finite": int((~np.isfinite(chapter_8[keep])).sum()), + } + + +NO_VIOLATIONS = {"negative": 0, "above_miscellaneous_income": 0, "not_finite": 0} + + +def test_frs_within_miscellaneous_income(frs): + if VARIABLE not in frs.person.columns: + pytest.skip(f"Dataset built before {VARIABLE} was added") + assert _bound_violations(frs.person) == NO_VIOLATIONS + assert int((frs.person[VARIABLE] > 0).sum()) > 0, "expected FRS odd-job income" + + +def test_enhanced_frs_within_bound_and_none_on_spi_donor_rows(enhanced_frs): + person = enhanced_frs.person + if VARIABLE not in person.columns: + pytest.skip(f"Dataset built before {VARIABLE} was added") + household = enhanced_frs.household + synthetic_households = household.household_id[ + household.household_is_spi_synthetic.astype(bool) + ] + synthetic = person.person_household_id.isin(synthetic_households).to_numpy() + assert int(synthetic.sum()) > 0, "expected SPI-donor rows in the enhanced FRS" + assert _bound_violations(person) == NO_VIOLATIONS + assert int((person[VARIABLE].to_numpy()[synthetic] != 0).sum()) == 0 + assert int((person[VARIABLE].to_numpy()[~synthetic] > 0).sum()) > 0 + + +def test_uprated_exactly_as_miscellaneous_income(): + from policyengine_uk.system import system + + if VARIABLE not in system.variables: + pytest.skip( + "The locked policyengine-uk does not read " + f"{VARIABLE} (PolicyEngine/policyengine-uk#2726)" + ) + table = pd.read_csv(STORAGE_FOLDER / "uprating_factors.csv", index_col="Variable") + assert VARIABLE in table.index, "regenerate uprating_factors.csv" + pd.testing.assert_series_equal( + table.loc[VARIABLE], table.loc["miscellaneous_income"], check_names=False + ) diff --git a/policyengine_uk_data/tests/test_non_negative_incomes.py b/policyengine_uk_data/tests/test_non_negative_incomes.py index 762d8fb0e..7b34be3a7 100644 --- a/policyengine_uk_data/tests/test_non_negative_incomes.py +++ b/policyengine_uk_data/tests/test_non_negative_incomes.py @@ -10,6 +10,7 @@ "property_income", "maintenance_income", "miscellaneous_income", + "income_not_otherwise_charged", ] From 3088545b3f2d831fa0068217e1c1fcdf001f2fc1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 29 Oct 2026 00:40:51 -0400 Subject: [PATCH 2/4] Keep record values out of data-test failures; check both bounds The data checks now reduce each dataset to counts in helper functions and assert only on those counts. Pytest's assertion rewriting printed the DataFrame argument of an assertion that failed, and the non-negative income test printed the smallest value: a single record's amount. The bound check now also counts negative and non-finite miscellaneous income, so a NaN there no longer passes, and a parametrised test covers each violation. The frs.py docstring now calls odd-job income a stand-in for Chapter 8 income: the FRS records it outside the employment and self-employment questions, but it does not say how HMRC would treat each job. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk_data/datasets/frs.py | 23 +++-- .../test_income_not_otherwise_charged.py | 93 ++++++++++++++----- .../tests/test_non_negative_incomes.py | 11 ++- 3 files changed, 90 insertions(+), 37 deletions(-) diff --git a/policyengine_uk_data/datasets/frs.py b/policyengine_uk_data/datasets/frs.py index e82607c28..8e3f713cc 100644 --- a/policyengine_uk_data/datasets/frs.py +++ b/policyengine_uk_data/datasets/frs.py @@ -381,16 +381,19 @@ def frs_miscellaneous_income( money the person received in the last 4 weeks from babysitting, acting as a mail order agent, or an odd job, occasional work or professional advice (ODDTYPE 1 to 3), that the employment and self-employment questions have - not covered. A person does an odd job once, without regularity or - continuity. OJAMT is the amount the person received and OJNOW whether - they did the work in the last 7 days. Jobs from the last 7 days count, as - weekly amounts (#640). Negative OJAMT values are FRS missing-value codes - (-1 to -9), not amounts, so they count as zero. - - Odd-job income is the income not otherwise charged: casual income that - no other provision charges, which ITTOIA 2005 Part 5 Chapter 8 charges - (s. 687) and the trading allowance relieves (s. 783AB). Casual work - that amounts to a trade takes the same allowance. The six other amounts + not covered. The FRS defines an odd job as one done once only, without + regularity or continuity. OJAMT is the amount the person received and + OJNOW whether they did the work in the last 7 days. Jobs from the last 7 + days count, as weekly amounts (#640). Negative OJAMT values are FRS + missing-value codes (-1 to -9), not amounts, so they count as zero. + + Odd-job income stands in for the income not otherwise charged. The FRS + records it outside the employment and self-employment questions, and + casual income that no other provision charges falls under ITTOIA 2005 + Part 5 Chapter 8 (s. 687), which the trading allowance relieves + (s. 783AB). The survey does not say how HMRC would treat each job: casual + work that amounts to a trade takes the same allowance as trade + receipts, and Chapter 8 then does not charge it. The six other amounts are not Chapter 8 income: royalties fall under Part 5 Chapter 2, a sleeping partner's share is partnership trading income, an overseas pension is pension income (ITEPA 2003 Part 9), a child's spare time job diff --git a/policyengine_uk_data/tests/test_income_not_otherwise_charged.py b/policyengine_uk_data/tests/test_income_not_otherwise_charged.py index ed4507092..30a62c77f 100644 --- a/policyengine_uk_data/tests/test_income_not_otherwise_charged.py +++ b/policyengine_uk_data/tests/test_income_not_otherwise_charged.py @@ -28,8 +28,9 @@ 7. The uprating table moves it exactly as miscellaneous income, so the bound holds in every year, once policyengine-uk reads it. -The data checks assert on counts only, so a failure never prints a record's -amounts (the FRS is licensed microdata). +The data checks reduce each dataset to counts in helper functions and +assert only on those counts, so a failure never prints a record (the FRS is +licensed microdata). """ import numpy as np @@ -207,40 +208,86 @@ def test_clear_adds_no_column_to_a_dataset_without_one(): assert VARIABLE not in cleared.person.columns -def _bound_violations(person, mask=None) -> dict: - chapter_8 = person[VARIABLE].to_numpy() - miscellaneous = person["miscellaneous_income"].to_numpy() - keep = np.ones(len(person), dtype=bool) if mask is None else mask +NO_VIOLATIONS = { + "negative": 0, + "not_finite": 0, + "negative_miscellaneous_income": 0, + "miscellaneous_income_not_finite": 0, + "above_miscellaneous_income": 0, +} + + +def _bound_violations(person) -> dict: + """Counts of rows that break invariant 1, in the keys of NO_VIOLATIONS.""" + chapter_8 = person[VARIABLE].to_numpy(dtype=float) + miscellaneous = person["miscellaneous_income"].to_numpy(dtype=float) return { - "negative": int((chapter_8[keep] < 0).sum()), - "above_miscellaneous_income": int((chapter_8 > miscellaneous)[keep].sum()), - "not_finite": int((~np.isfinite(chapter_8[keep])).sum()), + "negative": int((chapter_8 < 0).sum()), + "not_finite": int((~np.isfinite(chapter_8)).sum()), + "negative_miscellaneous_income": int((miscellaneous < 0).sum()), + "miscellaneous_income_not_finite": int((~np.isfinite(miscellaneous)).sum()), + "above_miscellaneous_income": int((chapter_8 > miscellaneous).sum()), } -NO_VIOLATIONS = {"negative": 0, "above_miscellaneous_income": 0, "not_finite": 0} +@pytest.mark.parametrize( + "chapter_8, miscellaneous, broken", + [ + (0.0, np.nan, ["miscellaneous_income_not_finite"]), + (np.nan, 10.0, ["not_finite"]), + (-1.0, 10.0, ["negative"]), + (0.0, -1.0, ["negative_miscellaneous_income", "above_miscellaneous_income"]), + (20.0, 10.0, ["above_miscellaneous_income"]), + (np.inf, np.inf, ["not_finite", "miscellaneous_income_not_finite"]), + ], +) +def test_bound_check_counts_each_violation(chapter_8, miscellaneous, broken): + person = pd.DataFrame( + {VARIABLE: [chapter_8, 5.0], "miscellaneous_income": [miscellaneous, 5.0]} + ) + assert _bound_violations(person) == {**NO_VIOLATIONS, **dict.fromkeys(broken, 1)} + + +def _frs_counts(dataset) -> dict: + person = dataset.person + return { + "violations": _bound_violations(person), + "with_value": int((person[VARIABLE] > 0).sum()), + } + + +def _enhanced_frs_counts(dataset) -> dict: + person = dataset.person + household = dataset.household + synthetic_households = household.household_id[ + household.household_is_spi_synthetic.astype(bool) + ] + synthetic = person.person_household_id.isin(synthetic_households).to_numpy() + values = person[VARIABLE].to_numpy(dtype=float) + return { + "violations": _bound_violations(person), + "spi_donor_rows": int(synthetic.sum()), + "spi_donor_rows_with_value": int((values[synthetic] != 0).sum()), + "frs_rows_with_value": int((values[~synthetic] > 0).sum()), + } def test_frs_within_miscellaneous_income(frs): if VARIABLE not in frs.person.columns: pytest.skip(f"Dataset built before {VARIABLE} was added") - assert _bound_violations(frs.person) == NO_VIOLATIONS - assert int((frs.person[VARIABLE] > 0).sum()) > 0, "expected FRS odd-job income" + counts = _frs_counts(frs) + assert counts["violations"] == NO_VIOLATIONS + assert counts["with_value"] > 0, "expected FRS odd-job income" def test_enhanced_frs_within_bound_and_none_on_spi_donor_rows(enhanced_frs): - person = enhanced_frs.person - if VARIABLE not in person.columns: + if VARIABLE not in enhanced_frs.person.columns: pytest.skip(f"Dataset built before {VARIABLE} was added") - household = enhanced_frs.household - synthetic_households = household.household_id[ - household.household_is_spi_synthetic.astype(bool) - ] - synthetic = person.person_household_id.isin(synthetic_households).to_numpy() - assert int(synthetic.sum()) > 0, "expected SPI-donor rows in the enhanced FRS" - assert _bound_violations(person) == NO_VIOLATIONS - assert int((person[VARIABLE].to_numpy()[synthetic] != 0).sum()) == 0 - assert int((person[VARIABLE].to_numpy()[~synthetic] > 0).sum()) > 0 + counts = _enhanced_frs_counts(enhanced_frs) + assert counts["spi_donor_rows"] > 0, "expected SPI-donor rows in the enhanced FRS" + assert counts["violations"] == NO_VIOLATIONS + assert counts["spi_donor_rows_with_value"] == 0 + assert counts["frs_rows_with_value"] > 0 def test_uprated_exactly_as_miscellaneous_income(): diff --git a/policyengine_uk_data/tests/test_non_negative_incomes.py b/policyengine_uk_data/tests/test_non_negative_incomes.py index 7b34be3a7..7131098fe 100644 --- a/policyengine_uk_data/tests/test_non_negative_incomes.py +++ b/policyengine_uk_data/tests/test_non_negative_incomes.py @@ -16,7 +16,10 @@ @pytest.mark.parametrize("variable", INCOME_VARIABLES) def test_income_non_negative(frs, variable: str): - """Test that income variables have no negative values.""" - values = frs.person[variable] - min_value = values.min() - assert min_value >= 0, f"{variable} has negative values (min = {min_value:.2f})" + """Test that income variables have no negative values. + + The test counts the negative values rather than reporting the smallest, so + a failure prints no single record's amount. + """ + negative = int((frs.person[variable] < 0).sum()) + assert negative == 0, f"{variable} has {negative} negative values" From ff0ccc3cca962a29d3c6223cfb6e5ccc56dcf0d4 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 29 Oct 2026 01:05:46 -0400 Subject: [PATCH 3/4] Fail data tests on errors without quoting a record An error inside a data test's reduction could still print records: pandas quotes an unconvertible value in its message, and under -l the traceback shows the DataFrame and arrays the reduction holds. counts_or_fail runs the reduction and turns any error into a failure that names only the function and the error's type, with no traceback, no chained exception and no record-bearing locals, even under --full-trace -l. Both data tests in test_income_not_otherwise_charged.py and the non-negative income test now reduce through it; test_record_safe.py checks it, warnings raised as errors included. The test module's summary and the changelog now call odd-job income a proxy for Chapter 8 income. Co-Authored-By: Claude Opus 5.5 --- .../income-not-otherwise-charged.added.md | 2 +- policyengine_uk_data/tests/record_safe.py | 33 +++++++++++ .../test_income_not_otherwise_charged.py | 15 ++--- .../tests/test_non_negative_incomes.py | 8 ++- .../tests/test_record_safe.py | 55 +++++++++++++++++++ 5 files changed, 104 insertions(+), 9 deletions(-) create mode 100644 policyengine_uk_data/tests/record_safe.py create mode 100644 policyengine_uk_data/tests/test_record_safe.py diff --git a/changelog.d/income-not-otherwise-charged.added.md b/changelog.d/income-not-otherwise-charged.added.md index ad95d1c79..5dc6053c2 100644 --- a/changelog.d/income-not-otherwise-charged.added.md +++ b/changelog.d/income-not-otherwise-charged.added.md @@ -1 +1 @@ -Set `income_not_otherwise_charged` to FRS odd-job income (ITTOIA 2005 Part 5 Chapter 8), so policyengine-uk's trading allowance relieves it. The other parts of `miscellaneous_income` (royalties, a sleeping partner's share, overseas pensions, a child's spare-time earnings, trust income and an organisation's allowance) stay outside it, and the SPI-synthetic copy holds none. +Set `income_not_otherwise_charged` to FRS odd-job income, as a proxy for income charged under ITTOIA 2005 Part 5 Chapter 8, so policyengine-uk's trading allowance relieves it. The other parts of `miscellaneous_income` (royalties, a sleeping partner's share, overseas pensions, a child's spare-time earnings, trust income and an organisation's allowance) stay outside it, and the SPI-synthetic copy holds none. diff --git a/policyengine_uk_data/tests/record_safe.py b/policyengine_uk_data/tests/record_safe.py new file mode 100644 index 000000000..8ce903454 --- /dev/null +++ b/policyengine_uk_data/tests/record_safe.py @@ -0,0 +1,33 @@ +"""Reduce licensed microdata to counts without letting a failure print a record. + +A data test that fails can print the values it checks: pytest shows the +operands of a failing assert, ``-l`` shows each frame's locals, and pandas and +numpy quote the offending value in some error messages ("could not convert +string to float: ..."). Tests on the FRS reduce a dataset to counts through +``counts_or_fail`` and assert only on what it returns. +""" + +import pytest + + +def counts_or_fail(count, dataset, *args): + """``count(dataset, *args)``, or a test failure that quotes no record. + + An error inside ``count`` becomes a failure that names only ``count`` and + the error's type. The failure carries no traceback (``pytrace=False``), + and the function raises it after the ``except`` block, so it chains no + exception and, under ``--full-trace -l``, the only locals it shows are + ``count``, ``dataset``, ``args`` and the type's name. Warnings that + ``-W error`` turns into exceptions take the same path. ``dataset`` and + ``args`` must not show records in their reprs (a UKSingleYearDataset + shows only its class and address). + """ + try: + return count(dataset, *args) + except Exception as error: + error_type = type(error).__name__ + raise pytest.fail.Exception( + f"{count.__name__} raised {error_type}; its message is withheld " + "because it can quote a record", + pytrace=False, + ) diff --git a/policyengine_uk_data/tests/test_income_not_otherwise_charged.py b/policyengine_uk_data/tests/test_income_not_otherwise_charged.py index 30a62c77f..93222fc51 100644 --- a/policyengine_uk_data/tests/test_income_not_otherwise_charged.py +++ b/policyengine_uk_data/tests/test_income_not_otherwise_charged.py @@ -1,10 +1,10 @@ -"""FRS odd-job income is the income not otherwise charged. +"""FRS odd-job income stands in for the income not otherwise charged. policyengine-uk's trading allowance relieves income chargeable under ITTOIA 2005 Part 5 Chapter 8 (s. 783AB), which it reads from ``income_not_otherwise_charged``, the part of ``miscellaneous_income`` that Chapter 8 charges. The build sets it to the FRS odd-job income in -``miscellaneous_income``. +``miscellaneous_income``, as a proxy for that part. Invariants, for any person table and ODDJOB table (amounts including FRS missing-value codes and blanks): @@ -28,9 +28,9 @@ 7. The uprating table moves it exactly as miscellaneous income, so the bound holds in every year, once policyengine-uk reads it. -The data checks reduce each dataset to counts in helper functions and -assert only on those counts, so a failure never prints a record (the FRS is -licensed microdata). +The data checks reduce each dataset to counts through ``counts_or_fail`` +and assert only on those counts, so neither a failed assertion nor an error +inside the reduction prints a record (the FRS is licensed microdata). """ import numpy as np @@ -49,6 +49,7 @@ clear_income_not_otherwise_charged, ) from policyengine_uk_data.storage import STORAGE_FOLDER +from policyengine_uk_data.tests.record_safe import counts_or_fail from policyengine_uk_data.utils.datasets import ( sum_from_positive_fields, sum_to_entity, @@ -275,7 +276,7 @@ def _enhanced_frs_counts(dataset) -> dict: def test_frs_within_miscellaneous_income(frs): if VARIABLE not in frs.person.columns: pytest.skip(f"Dataset built before {VARIABLE} was added") - counts = _frs_counts(frs) + counts = counts_or_fail(_frs_counts, frs) assert counts["violations"] == NO_VIOLATIONS assert counts["with_value"] > 0, "expected FRS odd-job income" @@ -283,7 +284,7 @@ def test_frs_within_miscellaneous_income(frs): def test_enhanced_frs_within_bound_and_none_on_spi_donor_rows(enhanced_frs): if VARIABLE not in enhanced_frs.person.columns: pytest.skip(f"Dataset built before {VARIABLE} was added") - counts = _enhanced_frs_counts(enhanced_frs) + counts = counts_or_fail(_enhanced_frs_counts, enhanced_frs) assert counts["spi_donor_rows"] > 0, "expected SPI-donor rows in the enhanced FRS" assert counts["violations"] == NO_VIOLATIONS assert counts["spi_donor_rows_with_value"] == 0 diff --git a/policyengine_uk_data/tests/test_non_negative_incomes.py b/policyengine_uk_data/tests/test_non_negative_incomes.py index 7131098fe..e05edb7df 100644 --- a/policyengine_uk_data/tests/test_non_negative_incomes.py +++ b/policyengine_uk_data/tests/test_non_negative_incomes.py @@ -1,5 +1,7 @@ import pytest +from policyengine_uk_data.tests.record_safe import counts_or_fail + INCOME_VARIABLES = [ "employment_income", "self_employment_income", @@ -21,5 +23,9 @@ def test_income_non_negative(frs, variable: str): The test counts the negative values rather than reporting the smallest, so a failure prints no single record's amount. """ - negative = int((frs.person[variable] < 0).sum()) + negative = counts_or_fail(_negative_values, frs, variable) assert negative == 0, f"{variable} has {negative} negative values" + + +def _negative_values(dataset, variable: str) -> int: + return int((dataset.person[variable] < 0).sum()) diff --git a/policyengine_uk_data/tests/test_record_safe.py b/policyengine_uk_data/tests/test_record_safe.py new file mode 100644 index 000000000..4c98b6f64 --- /dev/null +++ b/policyengine_uk_data/tests/test_record_safe.py @@ -0,0 +1,55 @@ +import warnings + +import numpy as np +import pytest + +from policyengine_uk_data.tests.record_safe import counts_or_fail + +SENTINEL = "777.25" + + +def _negative_values(table, column): + return int((table[column] < 0).sum()) + + +def _quotes_a_record(table): + raise ValueError(f"could not convert string to float: '{SENTINEL} pounds'") + + +def _warns_quoting_a_record(table): + warnings.warn(f"odd value {SENTINEL}", RuntimeWarning) + return 0 + + +def test_returns_the_counts(): + table = {"amount": np.array([-1.0, 2.0, -3.0])} + assert counts_or_fail(_negative_values, table, "amount") == 2 + + +@pytest.mark.parametrize( + "count, args", + [ + (_quotes_a_record, ()), + (_negative_values, ("missing_column",)), + ], +) +def test_an_error_fails_without_its_message(count, args): + table = {"amount": np.array([float(SENTINEL)])} + with pytest.raises(pytest.fail.Exception) as failure: + counts_or_fail(count, table, *args) + message = str(failure.value) + assert message.startswith(f"{count.__name__} raised ") + assert SENTINEL not in message + assert failure.value.pytrace is False + assert failure.value.__cause__ is None + assert failure.value.__context__ is None + + +def test_a_warning_raised_as_an_error_fails_without_its_message(): + with warnings.catch_warnings(): + warnings.simplefilter("error") + with pytest.raises(pytest.fail.Exception) as failure: + counts_or_fail(_warns_quoting_a_record, {}) + message = str(failure.value) + assert message.startswith("_warns_quoting_a_record raised RuntimeWarning") + assert SENTINEL not in message From 30e9cc574e9a2a9aba9ace86c126b46fe38eb980 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Thu, 29 Oct 2026 01:28:00 -0400 Subject: [PATCH 4/4] Say what counts_or_fail does to tracebacks precisely pytrace=False suppresses pytest's usual traceback, but --full-trace overrides it; the docstring now says so, and that the frames it then shows hold only the function, the dataset, the arguments and the error type's name. Co-Authored-By: Claude Opus 5.5 --- policyengine_uk_data/tests/record_safe.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/policyengine_uk_data/tests/record_safe.py b/policyengine_uk_data/tests/record_safe.py index 8ce903454..1c115d193 100644 --- a/policyengine_uk_data/tests/record_safe.py +++ b/policyengine_uk_data/tests/record_safe.py @@ -14,13 +14,14 @@ def counts_or_fail(count, dataset, *args): """``count(dataset, *args)``, or a test failure that quotes no record. An error inside ``count`` becomes a failure that names only ``count`` and - the error's type. The failure carries no traceback (``pytrace=False``), - and the function raises it after the ``except`` block, so it chains no - exception and, under ``--full-trace -l``, the only locals it shows are - ``count``, ``dataset``, ``args`` and the type's name. Warnings that - ``-W error`` turns into exceptions take the same path. ``dataset`` and - ``args`` must not show records in their reprs (a UKSingleYearDataset - shows only its class and address). + the error's type, and the error's own traceback goes with it. The function + raises the failure after the ``except`` block, so it chains no exception. + ``pytrace=False`` suppresses pytest's usual traceback; ``--full-trace`` + overrides that and shows the frames down to this function, whose only + locals are ``count``, ``dataset``, ``args`` and the type's name. + Warnings that ``-W error`` turns into exceptions take the same path. + ``dataset`` and ``args`` must not show records in their reprs (a + UKSingleYearDataset shows only its class and address). """ try: return count(dataset, *args)