[v4] Extract MicrosimulationModelVersion base class#294
Closed
MaxGhenis wants to merge 4 commits intov4-unify-program-statsfrom
Closed
[v4] Extract MicrosimulationModelVersion base class#294MaxGhenis wants to merge 4 commits intov4-unify-program-statsfrom
MaxGhenis wants to merge 4 commits intov4-unify-program-statsfrom
Conversation
Pulls ~300 lines of shared init/save/load logic out of PolicyEngineUSLatest and PolicyEngineUKLatest into a MicrosimulationModelVersion base in tax_benefit_models.common. The base handles: - Release-manifest fetch + installed-version warning - Data-release certification - Variable/parameter population from the country system - save() / load() + output-dataset filepath convention - _build_entity_relationships via declared group_entities Subclasses declare country_code, package_name, group_entities, entity_variables, and implement four thin hooks (_load_system, _load_region_registry, _dataset_class, _get_runtime_data_build_metadata). run() intentionally stays per-country: the US applies reforms at Microsimulation construction and manually copies structural columns, while the UK wraps inputs as UKSingleYearDataset and applies reforms after construction. Hiding those behind a shared skeleton would mask real divergence. Behaviour preservation is guarded by a byte-level snapshot test (tests/test_base_extraction_snapshot.py) covering four US and four UK household cases plus a model-surface snapshot. All 391 tests pass with zero snapshot drift.
2 tasks
MaxGhenis
force-pushed
the
v4-base-extraction
branch
from
7996660 to
b1fce7e
Compare
4 tasks
Contributor
Author
|
Superseded by #298 (consolidated v4 launch PR). All commits cherry-picked cleanly onto |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extracts ~300 lines of duplicated logic from
PolicyEngineUSLatestandPolicyEngineUKLatestinto a sharedMicrosimulationModelVersionbase class intax_benefit_models.common.What moves to the base:
systemsave()/load()+ output-dataset filepath convention_build_entity_relationshipsusing declaredgroup_entitiesWhat subclasses now declare (class-level):
country_code("us" / "uk")package_name("policyengine-us" / "policyengine-uk")group_entitiesentity_variablesPlus four thin hooks:
_load_system()— countrysystemobject_load_region_registry()— countryRegionRegistry_dataset_classproperty — countryPolicyEngine{Country}Dataset_get_runtime_data_build_metadata()— optional build-metadata dictrun()intentionally stays per-country. The US applies reforms atMicrosimulationconstruction and manually copies structural ID/weight columns; the UK wraps inputs asUKSingleYearDatasetand applies reforms via a modifier after construction. Hiding that behind a shared skeleton would mask real divergence.Safety
Behaviour preservation is guarded by
tests/test_base_extraction_snapshot.py— byte-level JSON snapshots for four US + four UK household cases plus a model-surface snapshot. Snapshots were frozen pre-refactor; they re-run clean post-refactor (zero drift).Stack
Stacked on #293 (unify
ProgramStatistics/ProgrammeStatistics). Base branch will retarget tomainonce #288-#293 merge.Test plan
pytest tests/green locally (391 passed)test_manifest_version_mismatchstill exercises the real warning branch (patch paths updated to the shared base module)🤖 Generated with Claude Code