Properly filter UK subnational simulation results#204
Conversation
The region parameter was being shadowed by a loop variable, causing the filtering logic to always use "northern_ireland" instead of the actual region parameter value. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![policyengine[bot]](https://avatars.githubusercontent.com/u/88570531?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed and fixed a critical bug in this PR.
What this PR does:
This PR properly filters UK constituency and local authority impact data when running simulations at subnational levels (country, constituency, or local authority). It introduces a new uk_geography.py utility module with filtering logic and applies it to zero out results for geographic areas outside the simulation region.
Bug found and fixed:
There was a variable shadowing bug in uk_constituency_breakdown() at line 711. The function parameter region was being shadowed by the loop variable for region in [...], which meant the filtering logic would always use "northern_ireland" (the last item in the loop) instead of the actual region parameter. I've fixed this by renaming the loop variable to region_.
What looks good:
- Clean separation of filtering logic into a dedicated utility module
- Comprehensive test coverage (333 lines of tests)
- The filtering logic correctly handles all region types (uk-wide, country, constituency, local_authority)
- Properly zeros out values and excludes them from aggregate counts
- Good use of type hints and docstrings
Testing:
The comprehensive test suite covers all the filtering scenarios. The fix I made ensures the filtering actually works as intended instead of defaulting to northern_ireland filtering.
With the fix committed, this PR properly addresses issue #203.
2 participants
Fixes #203
@PolicyEngine summarize this PR.