Skip to content

Rename labor to labour in LSR parameters#1607

Open
vahid-ahmadi wants to merge 3 commits intomainfrom
fix/labor-to-labour-lsr-853
Open

Rename labor to labour in LSR parameters#1607
vahid-ahmadi wants to merge 3 commits intomainfrom
fix/labor-to-labour-lsr-853

Conversation

@vahid-ahmadi
Copy link
Copy Markdown
Collaborator

Summary

  • Renames labor to labour (British English) across all LSR parameter and variable directory names, file names, labels, descriptions, import paths, and test references

Fixes #853

Test plan

  • Verify parameter paths resolve correctly (gov.simulation.labour_supply_responses.*)
  • Run LSR-related tests: pytest policyengine_uk/tests/test_labour_supply_response_formulas.py -v
  • Run behavioral response tests: pytest policyengine_uk/tests/test_behavioral_responses.py -v

🤖 Generated with Claude Code

vahid-ahmadi and others added 2 commits April 16, 2026 10:48
Use British English spelling throughout the labour supply response
module: directory names, file names, parameter labels, descriptions,
import paths, and test references. Fixes #853.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@MaxGhenis
Copy link
Copy Markdown
Collaborator

Clean rename but it's a breaking API change on an externally-referenced parameter path, so we can't merge standalone.

policyengine-app (the legacy app, still in production) hardcodes these two paths in src/pages/policy/PolicyRightSidebar.jsx:

  • gov.simulation.labor_supply_responses.income_elasticity
  • gov.simulation.labor_supply_responses.substitution_elasticity

Merging this as-is will silently break the LSR sliders in the policy sidebar the next time the app rebuilds against a policyengine-uk that has these parameters at labour_supply_responses.*.

Options to unblock:

  1. Cross-repo rename. Land a companion policyengine-app PR that updates those two path strings, merge both together. Simplest if we accept one short window of breakage on the deployed app between the two merges.
  2. Deprecation alias. Add gov.simulation.labor_supply_responses as an alias pointing to the renamed tree (or keep the old paths live for one release cycle), then remove in a later PR once consumers have migrated. Slower but zero external breakage.

I'd lean toward (1) — the app reference is only two lines and nothing else external uses these paths (I checked policyengine-api, policyengine-app-v2, policyengine-uk-data, policyengine-core). If you want, I can stage the companion app PR and link it here.

Blocking on that decision.

vahid-ahmadi added a commit to PolicyEngine/policyengine-app that referenced this pull request Apr 17, 2026
The UK labour supply response parameters are being renamed from
gov.simulation.labor_supply_responses.* to
gov.simulation.labour_supply_responses.* in PolicyEngine/policyengine-uk#1607.
Update the two UK-only path strings in PolicyRightSidebar so the LSR
sliders keep resolving after that merges. US paths unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vahid-ahmadi
Copy link
Copy Markdown
Collaborator Author

Going with option 1. Opened the companion app PR: PolicyEngine/policyengine-app#2829.

It updates just the two UK-only path strings in src/pages/policy/PolicyRightSidebar.jsx (income_elasticity, substitution_elasticity) — US paths are untouched since they live in policyengine-us and aren't being renamed here.

Suggested merge order to minimize the breakage window:

  1. Merge the app PR first so the frontend expects the new paths.
  2. Merge this PR, then bump policyengine-uk in the app.

Ready for review here once you're happy with the app-side change.

@vahid-ahmadi vahid-ahmadi self-assigned this Apr 17, 2026
@MaxGhenis
Copy link
Copy Markdown
Collaborator

Blocked on downstream coordination. policyengine-app/src/pages/policy/PolicyRightSidebar.jsx has two hardcoded references to gov.simulation.labor_supply_responses.income_elasticity and .substitution_elasticity that drive the UK labour-supply elasticity sliders. Merging this rename in isolation will break those sliders in the app.

Options to unblock:

  1. Deprecation alias — keep the old labor_supply_responses path as a read-only alias that forwards to labour_supply_responses for one release, then remove. Ships immediately; gives app/api time to migrate.
  2. Coordinated PR pair — file a matching PR in policyengine-app updating PolicyRightSidebar.jsx; merge them simultaneously. Cleanest end-state but the two repos can't be merged atomically so there will be a brief window where the app is broken against the latest uk.
  3. US-only keeps labor_ — no, the file is UK-only so this doesn't apply.

Preference would be (1): ship a parameter alias in this PR. policyengine-core supports parameter-path aliasing via node-level mirroring; I can help with that if useful. Otherwise (2) with the app PR filed first and ready to merge.

Happy to approve and merge as soon as one of those is in place.

Mirror the two consumed paths (`income_elasticity` and
`substitution_elasticity`) under the renamed
`labour_supply_responses` tree back to
`gov.simulation.labor_supply_responses.*`, so the legacy
policyengine-app PolicyRightSidebar (which hard-codes the old
paths) keeps working between this PR landing and the matching app
PR rebuilding against the new path. Removes the merge-window
breakage that would otherwise hit the LSR sliders.

Remove this alias in a follow-up release once consumers (the app
PR PolicyEngine/policyengine-app#2829) have migrated.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vahid-ahmadi
Copy link
Copy Markdown
Collaborator Author

Pushed 81f84a4 to take Max's option (1).

  • New module policyengine_uk/parameters/gov/simulation/labour_supply_responses/aliases.py mirrors the two consumed paths (income_elasticity, substitution_elasticity) back to gov.simulation.labor_supply_responses.*.
  • Wired in via process_parameters() in tax_benefit_system.py, after the rest of the parameter pipeline.
  • Verified both paths resolve to the same value:
    canonical labour: 0
    legacy labor:    0
    
  • Updated the changelog entry to call out the alias and the planned one-release deprecation window.
  • Scoped narrowly to the two paths PolicyRightSidebar actually consumes — not the whole legacy tree — so cleanup is a one-line node removal once policyengine-app rebuilds with Use British spelling for UK LSR parameter paths policyengine-app#2829.

Merge order is now decoupled — this PR can land first or last without breaking the deployed app's LSR sliders.

@vahid-ahmadi vahid-ahmadi requested a review from MaxGhenis April 21, 2026 11:14
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.

labor -> labour in LSR parameters

2 participants