Depend on policyengine[uk] instead of flooring policyengine-uk separately - #77
Open
vahid-ahmadi wants to merge 1 commit into
Open
Depend on policyengine[uk] instead of flooring policyengine-uk separately#77vahid-ahmadi wants to merge 1 commit into
vahid-ahmadi wants to merge 1 commit into
Conversation
…tely policyengine-uk was declared independently as >=2.89.2 with no upper bound, alongside policyengine>=4.22. That combination is unsound in a way that is easy to miss: `uk_latest` -- which _get_micro_data passes as tax_benefit_model_version -- reports the version from policyengine's bundled manifest, not from the installed policyengine-uk. So raising the standalone floor cannot raise the model version actually simulated against; it only decouples the installed rules from the manifest. With policyengine==5.0.4 and policyengine-uk==2.93.1 (both resolvable under the old declaration) the imports all work, but: UserWarning: Installed policyengine-uk version (2.93.1) does not match the bundled policyengine.py manifest (2.90.2). Calculations will run against the installed version, but dataset compatibility is not guaranteed. >>> uk_latest.version '2.90.2' policyengine-core drifted too (3.31.1 installed against the 3.30.1 the uk extra pins). Depending on the extra keeps all three in lockstep by construction. Nothing in oguk/ imports policyengine_uk directly, so the country package is an implementation detail of policyengine. Also caps the umbrella package below 6, rather than leaving the previous open-ended >=4.22 which silently admitted the 4.x -> 5.x major bump. uv.lock is unchanged: it already resolved to 5.0.4 / 3.30.1 / 2.90.2, so CI was testing this combination -- the declaration was the loose part. Refs PSLmodels#74 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XCKMb1aicxYaeUC1us2nvF
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.
Why
The standalone
policyengine-uk>=2.89.2floor looks like it controls the model version. It doesn't.uk_latest— passed astax_benefit_model_versionin_get_micro_data(api.py:378) — reads the version from policyengine's bundled manifest, not from the installedpolicyengine-uk. So the independent floor cannot raise the version OG-UK simulates against; it only lets the installed rules drift away from the manifest.With
policyengine==5.0.4andpolicyengine-uk==2.93.1, both admitted today, pip resolves happily and every import works — but:policyengine-coredrifts the same way (3.31.1 installed vs the 3.30.1 the extra pins).What changes after merging
A fresh unlocked install can no longer land mismatched rules against the manifest — the extra pins them together.
policyengineis also capped below 6, where before>=4.22silently admitted the 4.x→5.x major bump.Nothing moves in practice:
uv.lockis unchanged. It already resolved to 5.0.4 / 3.30.1 / 2.90.2, so CI has been testing this combination all along and only the declaration was loose.Change
Nothing in
oguk/importspolicyengine_ukdirectly, so the country package is an implementation detail ofpolicyengine.Evidence
Fresh install of
policyengine[uk]>=5.0,<6:Not verified: a full calibration run against 5.x — that needs microdata I don't have locally and CI doesn't exercise a solve. Since
uv.lockis unchanged this shouldn't move any resolved version, but a maintainer confirming onecalibrate()before merge would be worth it.Fixes #74.