Fix intra-decile income change double-counting#239
Merged
Conversation
…anges The intra_decile_impact and intra_wealth_decile_impact functions computed capped_reform_income as max(reform, 1) + absolute_change, which double-counted the change since reform already includes the change relative to baseline. Extracted a shared compute_income_change helper that correctly computes absolute_change / max(baseline, 1). Added 12 regression tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
calculate_income_specific_decile_winners_losers had the identical capped_reform_income = max(reform, 1) + absolute_change bug. Currently unreachable but fixing for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…name tests - Prefix compute_income_change with underscore (internal helper) - Move mock factory and constants to tests/fixtures/test_intra_decile.py - Rename all tests to test__given_X__Y naming convention Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 0.x versioning.yaml was also triggering on main pushes, which is redundant since main has its own copy. Removes main from the trigger branches to avoid conflicts. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The main branch Makefile uses towncrier for changelog generation but the workflow only installed yaml-changelog, causing 'towncrier: not found'. Install both so the workflow works for both 0.x (yaml-changelog) and main (towncrier). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
anth-volk
added a commit
to PolicyEngine/policyengine-api-v2
that referenced
this pull request
Feb 24, 2026
Picks up the fix for intra-decile income change double-counting (PolicyEngine/policyengine.py#239). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 tasks
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.
Fixes #238
Fixes #240
Summary
_compute_income_change(baseline_values, reform_values)helper that correctly computes(reform - baseline) / max(baseline, 1)intra_decile_impactandintra_wealth_decile_impactthat double-counted income changes by addingabsolute_changeto an already-changedreform_incomedecile.py(calculate_income_specific_decile_winners_losers, currently dead code)tests/fixtures/test_intra_decile.pyyaml-changelogandtowncrierin the versioning workflow so it works for both0.x(yaml-changelog) andmain(towncrier) branchesPorts the intra-decile fix from policyengine-api#3283 to the
0.xmaintenance branch.Test plan
pytest tests/test_intra_decile.py— all 12 tests pass0.xbranch🤖 Generated with Claude Code