Skip to content

Add UK detailed budget program statistics#359

Merged
anth-volk merged 4 commits into
mainfrom
add-uk-detailed-budget-program-statistics
May 19, 2026
Merged

Add UK detailed budget program statistics#359
anth-volk merged 4 commits into
mainfrom
add-uk-detailed-budget-program-statistics

Conversation

@anth-volk
Copy link
Copy Markdown
Contributor

Fixes #352

Summary

  • Expand UK ProgramStatistics coverage for detailed-budget parity by adding fuel_duty, state_pension, ni_employer, and tax_credits.
  • Add those UK variables to the default model output materialization list so analysis outputs can calculate them without caller-supplied extra_variables.
  • Validate UK program-statistics configuration before running analysis, with clearer errors for missing model variables or missing materialized outputs.
  • Add focused UK program-statistics tests, including a public economic_impact_analysis() wiring test with non-program outputs stubbed.

Notes

  • tax_credits intentionally overlaps with working_tax_credit and child_tax_credit; downstream budget adapters should choose the rows they need rather than summing all UK program-statistics rows.
  • This PR does not add a full UK integration simulation run. The tests use mocked output datasets to keep PR runtime bounded.

Tests

  • uv run --extra dev python -m pytest tests/test_uk_program_statistics.py tests/test_us_program_statistics.py -q
  • uv run ruff check tests/test_uk_program_statistics.py src/policyengine/tax_benefit_models/uk/analysis.py src/policyengine/tax_benefit_models/uk/model.py
  • uv run ruff format --check tests/test_uk_program_statistics.py src/policyengine/tax_benefit_models/uk/analysis.py src/policyengine/tax_benefit_models/uk/model.py

@anth-volk anth-volk requested a review from vahid-ahmadi May 13, 2026 22:23
Copy link
Copy Markdown
Contributor Author

@vahid-ahmadi I requested your review here. Could you specifically check whether the UK programs added in this PR align with the programs currently in use in the UK model/reporting flow, especially the tax_credits row and its relationship to working_tax_credit and child_tax_credit? The PR currently documents that these rows overlap and expects downstream adapters to choose the row set they need rather than summing all of them.

@anth-volk anth-volk marked this pull request as ready for review May 13, 2026 22:26
@anth-volk anth-volk requested review from PavelMakarchuk and removed request for vahid-ahmadi May 15, 2026 13:43
Copy link
Copy Markdown
Collaborator

@PavelMakarchuk PavelMakarchuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The four new programs (fuel_duty, state_pension, ni_employer, tax_credits) bring economic_impact_analysis(...).program_statistics to parity with policyengine-app's UKDetailedPrograms schema (src/schemas/societyWideModules.js). Entities, is_tax flags, and entity_variables wiring all line up. ProgramStatistics.run() itself is unchanged — this is pure additive wiring plus a pre-flight validation helper.

Test pattern is a nice step up from #358: the mocked simulation builds real MicroDataFrame / UKYearData / PolicyEngineUKDataset objects, with monkeypatching limited to Simulation.ensure and the unrelated non-program outputs.

Non-blocking suggestions for follow-up:

  1. Hoist resolve_entity_variables(simulation) out of the program loop in _validate_program_statistics_config — it currently runs once per program × 2 simulations and returns the full dict each time.
  2. Tighten the UK_PROGRAMS type hintdict[str, dict[str, bool]] or a TypedDict would self-document.
  3. tax_credits overlap with working_tax_credit + child_tax_credit — matches the legacy app behavior (the app only consumes tax_credits from UKDetailedPrograms), but worth either dropping the disaggregated rows or marking them with an overlapping=True flag in the data structure rather than relying on the PR description comment alone. Today a downstream consumer that naively sums all program_statistics rows will double-count.
  4. _format_missing_program_variables is a one-liner used once — could be inlined.
  5. No example update in examples/ — small miss.
  6. Schema asymmetrywealth_decile_impacts/program-stats list remains UK-only on PolicyReformAnalysis. Same follow-up as flagged on #358.

@anth-volk anth-volk force-pushed the add-uk-detailed-budget-program-statistics branch from 80a4147 to 1646113 Compare May 19, 2026 15:27
@anth-volk anth-volk merged commit 7ee13d5 into main May 19, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Determine whether pre-1 detailed_budget maps to ProgramStatistics

2 participants