From ee8a31de5fb07a352eef1eafdee853e55f9fd72f Mon Sep 17 00:00:00 2001 From: subfleet Date: Sun, 11 Oct 2026 14:50:58 -0400 Subject: [PATCH 1/6] Route UK dated reforms through policyengine-uk's statutory pipeline Simulation(policy=...) now passes a UK policy's parameter values to policyengine-uk as an explicit-interval reform dict at construction, instead of editing the already-annualised parameters through a modifier afterwards. With policyengine-uk#2305, that writes the changes onto the statutory timeline and reprocesses them as current law (uprating, threshold indexation, 30 April fiscal-year annualisation), before the UC rebalancing inputs and parameter-gated structural reforms read them. Custom simulation modifiers still run after construction. pe.uk.calculate_household(reform=...) gives date keys the same reading as Simulation(policy=...): a YYYY-MM-DD key is an effective date, so an April date is no longer a one-day override that changes nothing. Dict policies accept YYYY-MM-DD.YYYY-MM-DD inclusive intervals, and reject bare years, which policyengine-uk and policyengine-core read differently. Fixes #572. Co-Authored-By: Claude Opus 5.5 --- changelog.d/572.fixed.md | 1 + docs/households.md | 6 +- docs/reforms.md | 14 ++- .../tax_benefit_models/common/reform.py | 60 +++++++++-- .../tax_benefit_models/uk/household.py | 19 +++- .../tax_benefit_models/uk/model.py | 39 ++++--- tests/test_dict_reforms_on_simulation.py | 83 +++++++++++++- tests/test_household_impact.py | 3 +- tests/test_uk_dated_reforms.py | 101 ++++++++++++++++++ 9 files changed, 290 insertions(+), 36 deletions(-) create mode 100644 changelog.d/572.fixed.md create mode 100644 tests/test_uk_dated_reforms.py diff --git a/changelog.d/572.fixed.md b/changelog.d/572.fixed.md new file mode 100644 index 00000000..11f085d2 --- /dev/null +++ b/changelog.d/572.fixed.md @@ -0,0 +1 @@ +Align UK household and population reform routes with policy effective dates and inclusive date intervals, reject ambiguous bare-year keys, and pass parameter reforms at country-model construction so dated changes are reprocessed when supported by policyengine-uk. diff --git a/docs/households.md b/docs/households.md index 9ac9e710..f83e3fc7 100644 --- a/docs/households.md +++ b/docs/households.md @@ -150,7 +150,7 @@ Scale parameters use bracket indexing: reform = {"gov.irs.credits.ctc.amount.base[0].amount": 3_000} ``` -Time-varying reforms use a nested dict of `YYYY-MM-DD → value`: +Time-varying reforms use a nested dict of date keys to values: ```python reform = { @@ -161,6 +161,10 @@ reform = { } ``` +For UK households, a `YYYY-MM-DD` key is effective from that date until the next value for the same parameter, or December 31, 2100 if no later value follows. A `YYYY-MM-DD.YYYY-MM-DD` key applies over that inclusive interval. Scalar values default to January 1 of `year`. + +UK households reject bare years such as `"2027"` or `2027`, because the underlying packages assign different meanings to them. Use `"2027-04-06.2028-04-05"` for one UK fiscal year or `"2027-01-01"` for an effective date. See [Reforms](reforms.md) for the shared `Simulation(policy=...)` and `Simulation(dynamic=...)` date contract. The US household route retains its country package's period-key handling. + Structural reforms (new variables, formula swaps) require the `Simulation` path — see [Reforms](reforms.md). ## Year diff --git a/docs/reforms.md b/docs/reforms.md index cb66dbd6..9fcd9361 100644 --- a/docs/reforms.md +++ b/docs/reforms.md @@ -29,7 +29,19 @@ reform = { } ``` -Dates that haven't passed become "from this date onward." Earlier dates replace the baseline schedule. +A `YYYY-MM-DD` key is an effective date: its value applies from that date until the next date in the same parameter's reform schedule. If no later value follows, the wrapper emits an interval ending on December 31, 2100. + +Use `YYYY-MM-DD.YYYY-MM-DD` for an inclusive interval. For example, this UK reform applies for fiscal year 2027 only: + +```python +reform = { + "gov.hmrc.income_tax.rates.uk[0].rate": { + "2027-04-06.2028-04-05": 0.21, + }, +} +``` + +`Simulation(policy=...)`, `Simulation(dynamic=...)`, and UK `calculate_household(reform=...)` accept these two date formats and reject an interval whose end precedes its start. They reject bare years such as `"2027"` or `2027`, because policyengine-uk reads them as fiscal year 2027 only, while policyengine-core's `Reform.from_dict` reads them as from 2027 onward. Use `"2027-04-06.2028-04-05"` for one UK fiscal year or `"2027-01-01"` for an effective date. The US household route retains its country package's period-key handling. ### Multiple changes diff --git a/src/policyengine/tax_benefit_models/common/reform.py b/src/policyengine/tax_benefit_models/common/reform.py index 60b564f4..414e020d 100644 --- a/src/policyengine/tax_benefit_models/common/reform.py +++ b/src/policyengine/tax_benefit_models/common/reform.py @@ -1,4 +1,4 @@ -"""Compile a simple reform dict into the format policyengine_core expects. +"""Compile a simple reform dict into country-model inputs or parameter values. Accepted shapes for the agent-facing API: @@ -10,6 +10,13 @@ # With explicit effective date(s). reform = {"gov.irs.deductions.salt.cap": {"2026-01-01": 0}} + # With an inclusive start and end date. + reform = { + "gov.hmrc.income_tax.rates.uk[0].rate": { + "2027-04-06.2028-04-05": 0.21, + }, + } + # Multiple parameters. reform = { "gov.irs.deductions.salt.cap": 0, @@ -25,9 +32,16 @@ dict uses the bracket-head path instead of ``[0].amount`` the ``ValueError`` will list the close match. -The compiled form is ``{param_path: {period: value}}`` — exactly what -``policyengine_us.Simulation(reform=...)`` / -``policyengine_uk.Simulation(reform=...)`` accept at construction. +The parameter-value compiler accepts ``YYYY-MM-DD`` as an effective date +with no end date, or ``YYYY-MM-DD.YYYY-MM-DD`` as an inclusive interval. +Other keys, including bare years, are rejected: policyengine-uk interprets +a bare year as one fiscal year, while policyengine-core interprets it as +an effective year continuing onward. Use ``2027-04-06.2028-04-05`` for one +UK fiscal year, or ``2027-01-01`` for an effective date. + +The low-level :func:`compile_reform` returns ``{param_path: {period: value}}`` +without interpreting period keys. Parameter-value routes convert values +to explicit intervals before passing them to the country model. Scalar reforms default to ``{year}-01-01`` so a caller running mid-year does not accidentally get a blended partial-year result. @@ -38,6 +52,7 @@ from __future__ import annotations import datetime +import re from collections.abc import Mapping from difflib import get_close_matches from typing import TYPE_CHECKING, Any, Optional @@ -103,8 +118,8 @@ def _reform_dict_to_parameter_values( """Compile a flat reform dict into a list of ``ParameterValue`` objects. Uses :func:`compile_reform` for path validation and effective-date - defaulting, then materialises each ``{path: {date: value}}`` pair - as an open-ended ``ParameterValue`` bound to a + defaulting, then materialises each effective date or inclusive interval + as a ``ParameterValue`` bound to a ``Parameter(name=path, tax_benefit_model_version=model_version)``. """ from policyengine.core.parameter import Parameter @@ -117,6 +132,33 @@ def _reform_dict_to_parameter_values( parameter_values: list[ParameterValue] = [] for path, date_to_value in compiled.items(): for effective_date, value in date_to_value.items(): + try: + if not re.fullmatch( + r"[0-9]{4}-[0-9]{2}-[0-9]{2}" + r"(?:\.[0-9]{4}-[0-9]{2}-[0-9]{2})?", + effective_date, + ): + raise ValueError("Invalid date format") + start, *stops = effective_date.split(".") + start_date = datetime.datetime.strptime(start, "%Y-%m-%d") + end_date = ( + datetime.datetime.strptime(stops[0], "%Y-%m-%d") if stops else None + ) + except ValueError as exc: + raise ValueError( + f"Invalid reform date key {effective_date!r}. Accepted formats are " + "'YYYY-MM-DD' (effective date) and " + "'YYYY-MM-DD.YYYY-MM-DD' (inclusive interval). Bare years are " + "ambiguous: policyengine-uk's reform dict reads '2027' as fiscal " + "year 2027 only, while policyengine-core's Reform.from_dict " + "reads it as from 2027 on. Use '2027-04-06.2028-04-05' for one " + "UK fiscal year, or '2027-01-01' for an effective date." + ) from exc + if end_date is not None and end_date < start_date: + raise ValueError( + f"Reform interval {effective_date!r} has an end date before " + "its start date." + ) data_type = type(value) if isinstance(value, (int, float, bool)) else float parameter_values.append( ParameterValue( @@ -125,8 +167,8 @@ def _reform_dict_to_parameter_values( tax_benefit_model_version=model_version, data_type=data_type, ), - start_date=datetime.datetime.strptime(effective_date, "%Y-%m-%d"), - end_date=None, + start_date=start_date, + end_date=end_date, value=value, ) ) @@ -160,7 +202,7 @@ def compile_reform_to_policy( """Compile a flat reform dict into a fully-assembled ``Policy``. Accepts the same ``{param.path: value}`` / - ``{param.path: {date: value}}`` shape as :func:`compile_reform`, + ``{param.path: {date_or_interval: value}}`` shape as :func:`compile_reform`, but returns a ready-to-use ``Policy`` with ``ParameterValue`` objects so ``Simulation(policy={...})`` works without hand-building ``Parameter`` / ``ParameterValue``. diff --git a/src/policyengine/tax_benefit_models/uk/household.py b/src/policyengine/tax_benefit_models/uk/household.py index 41832336..2c5f8d0c 100644 --- a/src/policyengine/tax_benefit_models/uk/household.py +++ b/src/policyengine/tax_benefit_models/uk/household.py @@ -26,13 +26,16 @@ from policyengine.tax_benefit_models.common import ( EntityResult, HouseholdResult, - compile_reform, dispatch_extra_variables, normalize_axes, validate_annual_household_inputs, values_for_entity, ) +from policyengine.tax_benefit_models.common.reform import ( + _reform_dict_to_parameter_values, +) from policyengine.utils.household_validation import validate_household_input +from policyengine.utils.parametric_reforms import reform_dict_from_parameter_values from .model import uk_latest @@ -129,9 +132,11 @@ def calculate_household( Must be non-empty. benunit, household: Optional per-entity overrides. year: Calendar year. Defaults to 2026. - reform: Optional reform dict. Scalar values default to - ``{year}-01-01``; invalid parameter paths raise with a - close-match suggestion. + reform: Optional reform dict. Scalars start at ``{year}-01-01``. + ``YYYY-MM-DD`` keys are effective dates; a value lasts until + the next effective date or the end of the model horizon. + ``YYYY-MM-DD.YYYY-MM-DD`` keys specify inclusive intervals. + Invalid parameter paths raise with a close-match suggestion. extra_variables: Flat list of extra UK variables to compute; the library dispatches each to its entity. axes: Optional household-calculator axes. Pass either the lower-level @@ -181,7 +186,11 @@ def calculate_household( names=extra_variables or [], ) output_columns = _default_output_columns(extra_by_entity) - reform_dict = compile_reform(reform, year=year, model_version=uk_latest) + reform_dict = reform_dict_from_parameter_values( + _reform_dict_to_parameter_values( + reform or {}, year=year, model_version=uk_latest + ) + ) normalized_axes = normalize_axes(axes=axes, year=year, model_version=uk_latest) axes_active = normalized_axes is not None diff --git a/src/policyengine/tax_benefit_models/uk/model.py b/src/policyengine/tax_benefit_models/uk/model.py index 02dfa7c1..502f9a87 100644 --- a/src/policyengine/tax_benefit_models/uk/model.py +++ b/src/policyengine/tax_benefit_models/uk/model.py @@ -167,7 +167,8 @@ def run(self, simulation: "Simulation") -> "Simulation": from policyengine_uk import Microsimulation from policyengine.utils.parametric_reforms import ( - simulation_modifier_from_parameter_values, + build_reform_dict, + merge_reform_dicts, ) assert isinstance(simulation.dataset, PolicyEngineUKDataset) @@ -208,23 +209,29 @@ def run(self, simulation: "Simulation") -> "Simulation": data_year_data=dataset.data_year_data, ) - microsim = Microsimulation(dataset=_policyengine_uk_input(dataset)) + policy = simulation.policy + dynamic = simulation.dynamic + reform_dict = merge_reform_dicts( + build_reform_dict(policy) + if policy and policy.simulation_modifier is None + else None, + build_reform_dict(dynamic) + if dynamic and dynamic.simulation_modifier is None + else None, + ) + # Construction lets policyengine-uk reprocess dated parameter changes. + # It also builds an internal baseline that its formulas read through + # simulation.baseline. _policyengine_uk_input gives this run fresh + # tables, so enum encoding cannot mutate the caller's dataset. + microsim = Microsimulation( + dataset=_policyengine_uk_input(dataset), reform=reform_dict + ) - if simulation.policy and simulation.policy.simulation_modifier is not None: - simulation.policy.simulation_modifier(microsim) - elif simulation.policy: - modifier = simulation_modifier_from_parameter_values( - simulation.policy.parameter_values - ) - modifier(microsim) + if policy and policy.simulation_modifier is not None: + policy.simulation_modifier(microsim) - if simulation.dynamic and simulation.dynamic.simulation_modifier is not None: - simulation.dynamic.simulation_modifier(microsim) - elif simulation.dynamic: - modifier = simulation_modifier_from_parameter_values( - simulation.dynamic.parameter_values - ) - modifier(microsim) + if dynamic and dynamic.simulation_modifier is not None: + dynamic.simulation_modifier(microsim) data = { "person": pd.DataFrame(), diff --git a/tests/test_dict_reforms_on_simulation.py b/tests/test_dict_reforms_on_simulation.py index b1781c1a..2b7dcee0 100644 --- a/tests/test_dict_reforms_on_simulation.py +++ b/tests/test_dict_reforms_on_simulation.py @@ -10,12 +10,19 @@ from __future__ import annotations +import datetime + import pytest pytest.importorskip("policyengine_us") import policyengine as pe from policyengine.core import Dynamic, Policy, Simulation +from policyengine.tax_benefit_models.common.reform import ( + _reform_dict_to_parameter_values, + compile_reform_to_dynamic, + compile_reform_to_policy, +) # ``us_test_dataset`` is registered globally via ``tests/conftest.py``. @@ -45,6 +52,7 @@ def test__dict_policy__then_compiled_to_policy_with_parameter_values( # Scalar reforms default the effective date to {year}-01-01. assert pv.start_date.year == 2026 assert pv.start_date.month == 1 + assert pv.end_date is None def test__dict_policy_with_effective_date__then_start_date_matches( self, tiny_dataset @@ -66,6 +74,7 @@ def test__dict_policy_with_effective_date__then_start_date_matches( "2026-07-01", "2027-01-01", ] + assert all(pv.end_date is None for pv in sim.policy.parameter_values) def test__unknown_parameter_path__raises_with_suggestion(self, tiny_dataset): with pytest.raises(ValueError) as exc: @@ -81,8 +90,6 @@ def test__unknown_parameter_path__raises_with_suggestion(self, tiny_dataset): assert "did you mean" in str(exc.value) def test__existing_policy_object_passes_through_unchanged(self, tiny_dataset): - import datetime - from policyengine.core import Parameter, ParameterValue existing = Policy( @@ -126,3 +133,75 @@ def test__dict_dynamic__then_compiled_to_dynamic(self, tiny_dataset): assert isinstance(sim.dynamic, Dynamic) assert len(sim.dynamic.parameter_values) == 1 assert sim.dynamic.parameter_values[0].value == 2_800 + + +class TestReformDictDates: + path = "gov.irs.credits.ctc.amount.base[0].amount" + + @pytest.mark.parametrize( + "compiler", [compile_reform_to_policy, compile_reform_to_dynamic] + ) + @pytest.mark.parametrize("stop", ["2027-04-06", "2028-04-05"]) + def test__interval__then_dates_are_preserved(self, compiler, stop): + reform = compiler( + {self.path: {f"2027-04-06.{stop}": 3_000}}, + year=2027, + model_version=pe.us.model, + ) + assert len(reform.parameter_values) == 1 + pv = reform.parameter_values[0] + assert pv.start_date == datetime.datetime(2027, 4, 6) + assert pv.end_date == datetime.datetime.strptime(stop, "%Y-%m-%d") + assert pv.value == 3_000 + + def test__interval_end_before_start__raises(self): + with pytest.raises(ValueError, match="end date before its start date"): + _reform_dict_to_parameter_values( + {self.path: {"2028-04-06.2027-04-05": 3_000}}, + year=2027, + model_version=pe.us.model, + ) + + @pytest.mark.parametrize("key", ["2027", 2027]) + def test__bare_year__raises_with_country_semantics(self, key): + with pytest.raises(ValueError) as exc: + _reform_dict_to_parameter_values( + {self.path: {key: 3_000}}, year=2027, model_version=pe.us.model + ) + message = str(exc.value) + assert "'YYYY-MM-DD'" in message + assert "'YYYY-MM-DD.YYYY-MM-DD'" in message + assert "Bare years are ambiguous" in message + assert "policyengine-uk's reform dict" in message + assert "fiscal year 2027 only" in message + assert "policyengine-core's Reform.from_dict" in message + assert "from 2027 on" in message + assert "'2027-04-06.2028-04-05'" in message + assert "'2027-01-01'" in message + + @pytest.mark.parametrize( + "key", + [ + "2027-4-6", + "2027-02-29", + "2027-04-06.2028-02-30", + "2027-04-06.2028-04-05.2029-04-05", + "2027-04", + "not-a-date", + ], + ) + def test__other_date_keys__raise_with_accepted_formats(self, key): + with pytest.raises(ValueError, match="Accepted formats"): + _reform_dict_to_parameter_values( + {self.path: {key: 3_000}}, year=2027, model_version=pe.us.model + ) + + @pytest.mark.parametrize("spec", [3_000, {"2027-01-01": 3_000}]) + def test__scalar_or_effective_date__then_value_is_open_ended(self, spec): + values = _reform_dict_to_parameter_values( + {self.path: spec}, year=2027, model_version=pe.us.model + ) + assert len(values) == 1 + assert values[0].start_date == datetime.datetime(2027, 1, 1) + assert values[0].end_date is None + assert values[0].value == 3_000 diff --git a/tests/test_household_impact.py b/tests/test_household_impact.py index b62656c8..175d50c8 100644 --- a/tests/test_household_impact.py +++ b/tests/test_household_impact.py @@ -67,10 +67,9 @@ def test__reform_changes_child_benefit__then_dict_compiles_and_applies(self): year=2026, reform={"gov.hmrc.child_benefit.amount.eldest": 50.0}, ) - # If the param path is valid the calc runs; if results differ the reform took. - # Accept either: the key thing is the reform dict was accepted without error. assert isinstance(reformed.benunit.child_benefit, float) assert isinstance(baseline.benunit.child_benefit, float) + assert reformed.benunit.child_benefit > baseline.benunit.child_benefit def test__monthly_year_period__then_raises_before_calculation(self): with pytest.raises(ValueError, match="Monthly periods are not supported"): diff --git a/tests/test_uk_dated_reforms.py b/tests/test_uk_dated_reforms.py new file mode 100644 index 00000000..78eb9d50 --- /dev/null +++ b/tests/test_uk_dated_reforms.py @@ -0,0 +1,101 @@ +"""UK household reform dates share the wrapper policy's effective-date reading.""" + +import pytest + +import policyengine as pe +import policyengine.tax_benefit_models.uk.household as uk_household + +INCOME_TAX_RATE = "gov.hmrc.income_tax.rates.uk[0].rate" + + +@pytest.mark.parametrize( + ("schedule", "expected"), + [ + (0.21, {"2027-01-01.2100-12-31": 0.21}), + ({"2027-04-06": 0.21}, {"2027-04-06.2100-12-31": 0.21}), + ( + {"2027-04-06.2028-04-05": 0.21}, + {"2027-04-06.2028-04-05": 0.21}, + ), + ( + {"2028-04-06": 0.22, "2027-04-06": 0.21}, + { + "2028-04-06.2100-12-31": 0.22, + "2027-04-06.2028-04-05": 0.21, + }, + ), + ], + ids=["scalar", "effective-date", "interval", "reverse-date-schedule"], +) +def test_household_constructor_receives_inclusive_ranges( + monkeypatch, schedule, expected +): + captured = {} + + class CapturingSimulation: + def __init__(self, *, situation, reform): + captured["reform"] = reform + + def calculate(self, variable, period, map_to): + return [0.0] + + monkeypatch.setattr("policyengine_uk.Simulation", CapturingSimulation) + monkeypatch.setattr( + uk_household, + "_default_output_columns", + lambda extra: {"person": ["income_tax"]}, + ) + + pe.uk.calculate_household( + people=[{"age": 40, "employment_income": 30_000}], + year=2027, + reform={INCOME_TAX_RATE: schedule}, + ) + + assert captured["reform"] == {INCOME_TAX_RATE: expected} + assert all("." in key for key in captured["reform"][INCOME_TAX_RATE]) + + +def test_household_reform_keeps_unknown_path_validation(monkeypatch): + def unexpected_simulation(**kwargs): + pytest.fail("An invalid reform should fail before simulation construction") + + monkeypatch.setattr("policyengine_uk.Simulation", unexpected_simulation) + + with pytest.raises(ValueError, match="did you mean"): + pe.uk.calculate_household( + people=[{"age": 40}], + year=2027, + reform={"gov.hmrc.income_tax.rates.uk[0].rat": 0.21}, + ) + + +def test_household_april_effective_date_changes_fiscal_year_income_tax(): + pytest.importorskip("policyengine_uk.utils.dated_reforms") + + inputs = {"people": [{"age": 40, "employment_income": 30_000}], "year": 2027} + baseline = pe.uk.calculate_household(**inputs) + reformed = pe.uk.calculate_household( + **inputs, + reform={INCOME_TAX_RATE: {"2027-04-06": 0.21}}, + ) + + assert reformed.person[0].income_tax - baseline.person[0].income_tax == pytest.approx( + 174.30, abs=0.01 + ) + + +@pytest.mark.parametrize(("year", "expected_change"), [(2027, 174.30), (2028, 0.0)]) +def test_household_interval_reform_expires_after_fiscal_year(year, expected_change): + pytest.importorskip("policyengine_uk.utils.dated_reforms") + + inputs = {"people": [{"age": 40, "employment_income": 30_000}], "year": year} + baseline = pe.uk.calculate_household(**inputs) + reformed = pe.uk.calculate_household( + **inputs, + reform={INCOME_TAX_RATE: {"2027-04-06.2028-04-05": 0.21}}, + ) + + assert reformed.person[0].income_tax - baseline.person[0].income_tax == pytest.approx( + expected_change, abs=0.01 + ) From 883cfcac3e7badd1347e3f4ab674d0480217760e Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 15:38:20 -0400 Subject: [PATCH 2/6] Format the UK dated-reform tests Co-Authored-By: Claude Opus 5.5 --- tests/test_uk_dated_reforms.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_uk_dated_reforms.py b/tests/test_uk_dated_reforms.py index 78eb9d50..85a7c4ba 100644 --- a/tests/test_uk_dated_reforms.py +++ b/tests/test_uk_dated_reforms.py @@ -80,9 +80,9 @@ def test_household_april_effective_date_changes_fiscal_year_income_tax(): reform={INCOME_TAX_RATE: {"2027-04-06": 0.21}}, ) - assert reformed.person[0].income_tax - baseline.person[0].income_tax == pytest.approx( - 174.30, abs=0.01 - ) + assert reformed.person[0].income_tax - baseline.person[ + 0 + ].income_tax == pytest.approx(174.30, abs=0.01) @pytest.mark.parametrize(("year", "expected_change"), [(2027, 174.30), (2028, 0.0)]) @@ -96,6 +96,6 @@ def test_household_interval_reform_expires_after_fiscal_year(year, expected_chan reform={INCOME_TAX_RATE: {"2027-04-06.2028-04-05": 0.21}}, ) - assert reformed.person[0].income_tax - baseline.person[0].income_tax == pytest.approx( - expected_change, abs=0.01 - ) + assert reformed.person[0].income_tax - baseline.person[ + 0 + ].income_tax == pytest.approx(expected_change, abs=0.01) From 7a1acc043552236618b1dfad1d534f7304257f25 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 17:44:20 -0400 Subject: [PATCH 3/6] Keep policy before dynamic, and accept unpadded dates as before Review fixes (GPT-6.1 Sol): - A custom policy modifier followed by a parametric dynamic ran after the dynamic, which joined the construction reform. The dynamic is now applied after that modifier, as it was before. - merge_reform_dicts replaced an existing period in place, so a later policy period could override the dynamic where they overlapped. Override periods now come after the base's. - The dict compiler accepts any date strptime('%Y-%m-%d') accepts, as it always has, including unpadded months and days. Tests cover all four policy/dynamic combinations on the population route, overlapping override precedence, and unpadded dates. Co-Authored-By: Claude Opus 5.5 --- .../tax_benefit_models/common/reform.py | 17 +- .../tax_benefit_models/uk/model.py | 47 +++-- src/policyengine/utils/parametric_reforms.py | 15 +- tests/test_dict_reforms_on_simulation.py | 19 ++- tests/test_uk_dated_reforms.py | 161 ++++++++++++++++++ 5 files changed, 229 insertions(+), 30 deletions(-) diff --git a/src/policyengine/tax_benefit_models/common/reform.py b/src/policyengine/tax_benefit_models/common/reform.py index 414e020d..afbc4583 100644 --- a/src/policyengine/tax_benefit_models/common/reform.py +++ b/src/policyengine/tax_benefit_models/common/reform.py @@ -52,7 +52,6 @@ from __future__ import annotations import datetime -import re from collections.abc import Mapping from difflib import get_close_matches from typing import TYPE_CHECKING, Any, Optional @@ -133,16 +132,14 @@ def _reform_dict_to_parameter_values( for path, date_to_value in compiled.items(): for effective_date, value in date_to_value.items(): try: - if not re.fullmatch( - r"[0-9]{4}-[0-9]{2}-[0-9]{2}" - r"(?:\.[0-9]{4}-[0-9]{2}-[0-9]{2})?", - effective_date, - ): - raise ValueError("Invalid date format") - start, *stops = effective_date.split(".") - start_date = datetime.datetime.strptime(start, "%Y-%m-%d") + parts = str(effective_date).split(".") + if len(parts) > 2: + raise ValueError("Too many dates") + start_date = datetime.datetime.strptime(parts[0], "%Y-%m-%d") end_date = ( - datetime.datetime.strptime(stops[0], "%Y-%m-%d") if stops else None + datetime.datetime.strptime(parts[1], "%Y-%m-%d") + if len(parts) == 2 + else None ) except ValueError as exc: raise ValueError( diff --git a/src/policyengine/tax_benefit_models/uk/model.py b/src/policyengine/tax_benefit_models/uk/model.py index 502f9a87..f6b65fdf 100644 --- a/src/policyengine/tax_benefit_models/uk/model.py +++ b/src/policyengine/tax_benefit_models/uk/model.py @@ -169,6 +169,7 @@ def run(self, simulation: "Simulation") -> "Simulation": from policyengine.utils.parametric_reforms import ( build_reform_dict, merge_reform_dicts, + simulation_modifier_from_parameter_values, ) assert isinstance(simulation.dataset, PolicyEngineUKDataset) @@ -211,27 +212,43 @@ def run(self, simulation: "Simulation") -> "Simulation": policy = simulation.policy dynamic = simulation.dynamic - reform_dict = merge_reform_dicts( - build_reform_dict(policy) - if policy and policy.simulation_modifier is None - else None, - build_reform_dict(dynamic) - if dynamic and dynamic.simulation_modifier is None - else None, + policy_modifier = policy.simulation_modifier if policy else None + dynamic_modifier = dynamic.simulation_modifier if dynamic else None + # Parameter values reach policyengine-uk as a reform dict at + # construction, which writes them onto the statutory timeline and + # reprocesses the parameters. Policy comes before dynamic, as it + # always has: a parametric dynamic joins the construction reform + # (merge_reform_dicts puts its values last, so they win) unless a + # custom policy modifier must run before it; then it is applied after + # that modifier, to the annualised parameters, as before. + reform_dict = ( + build_reform_dict(policy) if policy and policy_modifier is None else None ) - # Construction lets policyengine-uk reprocess dated parameter changes. - # It also builds an internal baseline that its formulas read through - # simulation.baseline. _policyengine_uk_input gives this run fresh - # tables, so enum encoding cannot mutate the caller's dataset. + late_dynamic = None + if dynamic and dynamic_modifier is None: + if policy_modifier is None: + reform_dict = merge_reform_dicts( + reform_dict, build_reform_dict(dynamic) + ) + else: + late_dynamic = dynamic + # policyengine-uk also builds an internal baseline from the same + # input, which its formulas read through simulation.baseline. + # _policyengine_uk_input gives this run fresh tables, so enum + # encoding cannot mutate the caller's dataset. microsim = Microsimulation( dataset=_policyengine_uk_input(dataset), reform=reform_dict ) - if policy and policy.simulation_modifier is not None: - policy.simulation_modifier(microsim) + if policy_modifier is not None: + policy_modifier(microsim) - if dynamic and dynamic.simulation_modifier is not None: - dynamic.simulation_modifier(microsim) + if dynamic_modifier is not None: + dynamic_modifier(microsim) + elif late_dynamic is not None and late_dynamic.parameter_values: + simulation_modifier_from_parameter_values(late_dynamic.parameter_values)( + microsim + ) data = { "person": pd.DataFrame(), diff --git a/src/policyengine/utils/parametric_reforms.py b/src/policyengine/utils/parametric_reforms.py index 58d2c1f2..4b33a2e4 100644 --- a/src/policyengine/utils/parametric_reforms.py +++ b/src/policyengine/utils/parametric_reforms.py @@ -131,7 +131,8 @@ def merge_reform_dicts( """Merge two reform dicts, with override values taking precedence. Either or both dicts can be None. When both have entries for the same - parameter, period-level values from override replace those in base. + parameter, period-level values from override replace those in base, and + every override period comes after the base's periods. Args: base: The base reform dict (e.g., from policy). @@ -147,7 +148,13 @@ def merge_reform_dicts( merged = {k: dict(v) for k, v in base.items()} for param_name, period_values in override.items(): - if param_name not in merged: - merged[param_name] = {} - merged[param_name].update(period_values) + # Override values go after the base's, so a country model that + # applies a parameter's periods in order lets them win where they + # overlap a base period with a different key. + kept = { + period: value + for period, value in merged.get(param_name, {}).items() + if period not in period_values + } + merged[param_name] = {**kept, **period_values} return merged diff --git a/tests/test_dict_reforms_on_simulation.py b/tests/test_dict_reforms_on_simulation.py index 2b7dcee0..b178c324 100644 --- a/tests/test_dict_reforms_on_simulation.py +++ b/tests/test_dict_reforms_on_simulation.py @@ -182,7 +182,6 @@ def test__bare_year__raises_with_country_semantics(self, key): @pytest.mark.parametrize( "key", [ - "2027-4-6", "2027-02-29", "2027-04-06.2028-02-30", "2027-04-06.2028-04-05.2029-04-05", @@ -205,3 +204,21 @@ def test__scalar_or_effective_date__then_value_is_open_ended(self, spec): assert values[0].start_date == datetime.datetime(2027, 1, 1) assert values[0].end_date is None assert values[0].value == 3_000 + + @pytest.mark.parametrize( + "key, start, end", + [ + ("2027-4-6", datetime.datetime(2027, 4, 6), None), + ( + "2027-4-6.2028-4-5", + datetime.datetime(2027, 4, 6), + datetime.datetime(2028, 4, 5), + ), + ], + ) + def test__unpadded_dates__then_accepted_as_before(self, key, start, end): + # The compiler has always accepted what strptime("%Y-%m-%d") does. + (value,) = _reform_dict_to_parameter_values( + {self.path: {key: 3_000}}, year=2027, model_version=pe.us.model + ) + assert (value.start_date, value.end_date) == (start, end) diff --git a/tests/test_uk_dated_reforms.py b/tests/test_uk_dated_reforms.py index 85a7c4ba..7c201f7e 100644 --- a/tests/test_uk_dated_reforms.py +++ b/tests/test_uk_dated_reforms.py @@ -99,3 +99,164 @@ def test_household_interval_reform_expires_after_fiscal_year(year, expected_chan assert reformed.person[0].income_tax - baseline.person[ 0 ].income_tax == pytest.approx(expected_change, abs=0.01) + + +def test_merged_override_periods_come_last_and_win_where_they_overlap(): + from policyengine.utils.parametric_reforms import merge_reform_dicts + + policy = { + INCOME_TAX_RATE: { + "2027-01-01.2100-12-31": 0.21, + "2028-01-01.2100-12-31": 0.22, + } + } + dynamic = {INCOME_TAX_RATE: {"2027-01-01.2100-12-31": 0.30}} + merged = merge_reform_dicts(policy, dynamic) + assert list(merged[INCOME_TAX_RATE].items()) == [ + ("2028-01-01.2100-12-31", 0.22), + ("2027-01-01.2100-12-31", 0.30), + ] + # The inputs are not changed. + assert list(policy[INCOME_TAX_RATE]) == [ + "2027-01-01.2100-12-31", + "2028-01-01.2100-12-31", + ] + + +class _StopRun(Exception): + pass + + +def _run_population_route(monkeypatch, policy, dynamic, events): + """Run the UK population route up to its first calculation.""" + import importlib + + import pandas as pd + from microdf import MicroDataFrame + + import policyengine.utils.parametric_reforms as parametric_reforms + from policyengine.core import Simulation + from policyengine.tax_benefit_models.uk.datasets import ( + PolicyEngineUKDataset, + UKYearData, + ) + + # The package's ``model`` attribute shadows the submodule's name. + uk_model = importlib.import_module("policyengine.tax_benefit_models.uk.model") + + class RecordingMicrosimulation: + def __init__(self, *, dataset, reform=None): + events.append(("construct", reform)) + + def calculate(self, *args, **kwargs): + raise _StopRun + + monkeypatch.setattr("policyengine_uk.Microsimulation", RecordingMicrosimulation) + monkeypatch.setattr(uk_model, "_policyengine_uk_input", lambda dataset: None) + monkeypatch.setattr( + parametric_reforms, + "simulation_modifier_from_parameter_values", + lambda values: lambda sim: events.append( + ("late parameter values", [(v.parameter.name, v.value) for v in values]) + ), + ) + + def table(columns, weights): + return MicroDataFrame(pd.DataFrame(columns), weights=weights) + + dataset = PolicyEngineUKDataset( + id="order", + name="order", + description="order", + filepath=None, + year=2028, + data=UKYearData( + person=table( + { + "person_id": [1], + "person_benunit_id": [1], + "person_household_id": [1], + "person_weight": [1.0], + }, + "person_weight", + ), + benunit=table( + {"benunit_id": [1], "benunit_weight": [1.0]}, "benunit_weight" + ), + household=table( + {"household_id": [1], "household_weight": [1.0]}, "household_weight" + ), + ), + ) + simulation = Simulation( + dataset=dataset, + tax_benefit_model_version=pe.uk.model, + policy=policy, + dynamic=dynamic, + ) + with pytest.raises(_StopRun): + pe.uk.model.run(simulation) + + +@pytest.mark.parametrize( + "policy_kind, dynamic_kind, expected", + [ + ( + "parametric", + "parametric", + [("construct", {INCOME_TAX_RATE: {"2027-04-06.2100-12-31": 0.30}})], + ), + ( + "parametric", + "custom", + [ + ("construct", {INCOME_TAX_RATE: {"2027-04-06.2100-12-31": 0.21}}), + "dynamic modifier", + ], + ), + ( + # The dynamic still follows the custom policy, as before #572. + "custom", + "parametric", + [ + ("construct", None), + "policy modifier", + ("late parameter values", [(INCOME_TAX_RATE, 0.30)]), + ], + ), + ( + "custom", + "custom", + [("construct", None), "policy modifier", "dynamic modifier"], + ), + ], +) +def test_population_route_applies_policy_before_dynamic( + monkeypatch, policy_kind, dynamic_kind, expected +): + from policyengine.core import Dynamic, Policy + from policyengine.tax_benefit_models.common.reform import ( + _reform_dict_to_parameter_values, + ) + + events = [] + + def make(cls, kind, value, name): + if kind == "custom": + return cls(name=name, simulation_modifier=lambda sim: events.append(name)) + return cls( + name=name, + parameter_values=_reform_dict_to_parameter_values( + {INCOME_TAX_RATE: {"2027-04-06": value}}, + year=2028, + model_version=pe.uk.model, + ), + ) + + _run_population_route( + monkeypatch, + make(Policy, policy_kind, 0.21, "policy modifier"), + make(Dynamic, dynamic_kind, 0.30, "dynamic modifier"), + events, + ) + assert events == expected From 28581aa97747bbc03ef3981c6954248b39e47029 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 18:51:54 -0400 Subject: [PATCH 4/6] Say what the merge order and date parsing actually do Review notes (GPT-6.1 Sol round 2): the merge docstring claimed the override always wins an overlap, which holds for policyengine-uk but not for policyengine-core's start-ordered Reform.from_dict on the US route; and the compiler's docs and error did not mention unpadded dates. Co-Authored-By: Claude Opus 5.5 --- src/policyengine/tax_benefit_models/common/reform.py | 4 +++- src/policyengine/utils/parametric_reforms.py | 11 +++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/policyengine/tax_benefit_models/common/reform.py b/src/policyengine/tax_benefit_models/common/reform.py index afbc4583..a2bbae09 100644 --- a/src/policyengine/tax_benefit_models/common/reform.py +++ b/src/policyengine/tax_benefit_models/common/reform.py @@ -34,6 +34,7 @@ The parameter-value compiler accepts ``YYYY-MM-DD`` as an effective date with no end date, or ``YYYY-MM-DD.YYYY-MM-DD`` as an inclusive interval. +Months and days may be unpadded (``2027-4-6``), as ``strptime`` reads them. Other keys, including bare years, are rejected: policyengine-uk interprets a bare year as one fiscal year, while policyengine-core interprets it as an effective year continuing onward. Use ``2027-04-06.2028-04-05`` for one @@ -145,7 +146,8 @@ def _reform_dict_to_parameter_values( raise ValueError( f"Invalid reform date key {effective_date!r}. Accepted formats are " "'YYYY-MM-DD' (effective date) and " - "'YYYY-MM-DD.YYYY-MM-DD' (inclusive interval). Bare years are " + "'YYYY-MM-DD.YYYY-MM-DD' (inclusive interval); months and " + "days may be unpadded. Bare years are " "ambiguous: policyengine-uk's reform dict reads '2027' as fiscal " "year 2027 only, while policyengine-core's Reform.from_dict " "reads it as from 2027 on. Use '2027-04-06.2028-04-05' for one " diff --git a/src/policyengine/utils/parametric_reforms.py b/src/policyengine/utils/parametric_reforms.py index 4b33a2e4..7f96025a 100644 --- a/src/policyengine/utils/parametric_reforms.py +++ b/src/policyengine/utils/parametric_reforms.py @@ -131,8 +131,15 @@ def merge_reform_dicts( """Merge two reform dicts, with override values taking precedence. Either or both dicts can be None. When both have entries for the same - parameter, period-level values from override replace those in base, and - every override period comes after the base's periods. + parameter, an override value replaces the base value with the same + period key, and every override period comes after the base's periods. + + Where an override period overlaps a base period with a different key, + the result depends on the country model. policyengine-uk applies a + parameter's periods in the order given, so the override wins. + policyengine-core's ``Reform.from_dict``, which the US route uses, + applies them in order of start date, so the later-starting period wins + whichever dict it came from. Args: base: The base reform dict (e.g., from policy). From 19ba06b4de1c944658c169b890ea890bf8b4ba32 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 19:32:00 -0400 Subject: [PATCH 5/6] Format the UK dated-reform tests as CI's ruff does Co-Authored-By: Claude Opus 5.5 --- tests/test_uk_dated_reforms.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/test_uk_dated_reforms.py b/tests/test_uk_dated_reforms.py index 7c201f7e..5ee73eec 100644 --- a/tests/test_uk_dated_reforms.py +++ b/tests/test_uk_dated_reforms.py @@ -156,8 +156,10 @@ def calculate(self, *args, **kwargs): monkeypatch.setattr( parametric_reforms, "simulation_modifier_from_parameter_values", - lambda values: lambda sim: events.append( - ("late parameter values", [(v.parameter.name, v.value) for v in values]) + lambda values: ( + lambda sim: events.append( + ("late parameter values", [(v.parameter.name, v.value) for v in values]) + ) ), ) From 0d4e9b81f2cc8f0ccd26fe69c82e3bc1adcb7f59 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 11 Oct 2026 20:56:47 -0400 Subject: [PATCH 6/6] Apply a UK policy's parameter values before its modifier, not instead of them A Policy or Dynamic sum carrying both parameter values and a simulation modifier lost its values on the UK population route: the modifier ran in their place, on main as on this branch. The values now go in at construction and the modifier runs after them, policy before dynamic. Co-Authored-By: Claude Opus 5.5 --- changelog.d/572.fixed.md | 2 +- .../tax_benefit_models/uk/model.py | 37 +++++++-------- tests/test_uk_dated_reforms.py | 45 ++++++++++++++++++- 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/changelog.d/572.fixed.md b/changelog.d/572.fixed.md index 11f085d2..201a61f6 100644 --- a/changelog.d/572.fixed.md +++ b/changelog.d/572.fixed.md @@ -1 +1 @@ -Align UK household and population reform routes with policy effective dates and inclusive date intervals, reject ambiguous bare-year keys, and pass parameter reforms at country-model construction so dated changes are reprocessed when supported by policyengine-uk. +Align UK household and population reform routes with policy effective dates and inclusive date intervals, reject ambiguous bare-year keys, and pass parameter reforms at country-model construction so dated changes are reprocessed when supported by policyengine-uk. A UK policy or dynamic carrying both parameter values and a simulation modifier (as a `Policy` or `Dynamic` sum does) now applies its values and then its modifier, where the modifier used to replace the values. diff --git a/src/policyengine/tax_benefit_models/uk/model.py b/src/policyengine/tax_benefit_models/uk/model.py index f6b65fdf..4aafcc0a 100644 --- a/src/policyengine/tax_benefit_models/uk/model.py +++ b/src/policyengine/tax_benefit_models/uk/model.py @@ -214,24 +214,22 @@ def run(self, simulation: "Simulation") -> "Simulation": dynamic = simulation.dynamic policy_modifier = policy.simulation_modifier if policy else None dynamic_modifier = dynamic.simulation_modifier if dynamic else None + # Each object's parameter values come before its modifier, and the + # policy before the dynamic: policy values, policy modifier, dynamic + # values, dynamic modifier. A modifier never replaces its object's + # parameter values (a Policy or Dynamic sum can carry both). # Parameter values reach policyengine-uk as a reform dict at # construction, which writes them onto the statutory timeline and - # reprocesses the parameters. Policy comes before dynamic, as it - # always has: a parametric dynamic joins the construction reform - # (merge_reform_dicts puts its values last, so they win) unless a - # custom policy modifier must run before it; then it is applied after - # that modifier, to the annualised parameters, as before. - reform_dict = ( - build_reform_dict(policy) if policy and policy_modifier is None else None - ) - late_dynamic = None - if dynamic and dynamic_modifier is None: - if policy_modifier is None: - reform_dict = merge_reform_dicts( - reform_dict, build_reform_dict(dynamic) - ) - else: - late_dynamic = dynamic + # reprocesses the parameters. The dynamic's values join that reform + # (merge_reform_dicts puts them last, so they win) unless a policy + # modifier must run before them; then they are applied after that + # modifier, to the annualised parameters, as before. + reform_dict = build_reform_dict(policy) + late_dynamic_values = None + if policy_modifier is None: + reform_dict = merge_reform_dicts(reform_dict, build_reform_dict(dynamic)) + elif dynamic is not None and dynamic.parameter_values: + late_dynamic_values = dynamic.parameter_values # policyengine-uk also builds an internal baseline from the same # input, which its formulas read through simulation.baseline. # _policyengine_uk_input gives this run fresh tables, so enum @@ -243,12 +241,11 @@ def run(self, simulation: "Simulation") -> "Simulation": if policy_modifier is not None: policy_modifier(microsim) + if late_dynamic_values is not None: + simulation_modifier_from_parameter_values(late_dynamic_values)(microsim) + if dynamic_modifier is not None: dynamic_modifier(microsim) - elif late_dynamic is not None and late_dynamic.parameter_values: - simulation_modifier_from_parameter_values(late_dynamic.parameter_values)( - microsim - ) data = { "person": pd.DataFrame(), diff --git a/tests/test_uk_dated_reforms.py b/tests/test_uk_dated_reforms.py index 5ee73eec..cfa27ab7 100644 --- a/tests/test_uk_dated_reforms.py +++ b/tests/test_uk_dated_reforms.py @@ -231,6 +231,43 @@ def table(columns, weights): "custom", [("construct", None), "policy modifier", "dynamic modifier"], ), + # A modifier never replaces its object's parameter values (#572: a + # Policy or Dynamic sum carrying both lost its values). + ( + "both", + "none", + [ + ("construct", {INCOME_TAX_RATE: {"2027-04-06.2100-12-31": 0.21}}), + "policy modifier", + ], + ), + ( + "both", + "parametric", + [ + ("construct", {INCOME_TAX_RATE: {"2027-04-06.2100-12-31": 0.21}}), + "policy modifier", + ("late parameter values", [(INCOME_TAX_RATE, 0.30)]), + ], + ), + ( + "parametric", + "both", + [ + ("construct", {INCOME_TAX_RATE: {"2027-04-06.2100-12-31": 0.30}}), + "dynamic modifier", + ], + ), + ( + "both", + "both", + [ + ("construct", {INCOME_TAX_RATE: {"2027-04-06.2100-12-31": 0.21}}), + "policy modifier", + ("late parameter values", [(INCOME_TAX_RATE, 0.30)]), + "dynamic modifier", + ], + ), ], ) def test_population_route_applies_policy_before_dynamic( @@ -244,9 +281,12 @@ def test_population_route_applies_policy_before_dynamic( events = [] def make(cls, kind, value, name): + if kind == "none": + return None + modifier = cls(name=name, simulation_modifier=lambda sim: events.append(name)) if kind == "custom": - return cls(name=name, simulation_modifier=lambda sim: events.append(name)) - return cls( + return modifier + parametric = cls( name=name, parameter_values=_reform_dict_to_parameter_values( {INCOME_TAX_RATE: {"2027-04-06": value}}, @@ -254,6 +294,7 @@ def make(cls, kind, value, name): model_version=pe.uk.model, ), ) + return parametric + modifier if kind == "both" else parametric _run_population_route( monkeypatch,