Add UK Parliamentary constituencies #2370
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Thanks for this @nikhilwoodruff. I attempted to run this locally, but using the deployed API (which now contains the API changes necessary to enable this feature), but after trying to run a standard reform (http://localhost:3000/uk/policy?focus=policyOutput.policyBreakdown&reform=76682®ion=uk&timePeriod=2025&baseline=1), I get a "Sorry, something went wrong", and the network tab shows that there is a calculation error somewhere on the back end.
|
Thanks @anth-volk - I think it might be fixed by this PR. |
|
Also adding to this the feedback from @MaxGhenis that we should split out the winner/loser chart by UK country. |
There was a problem hiding this comment.
- Running this code against the prod API with a reform over a country (e.g., http://localhost:3000/uk/policy?focus=policyOutput.povertyImpact.regular.byAge&reform=76756®ion=country%2Fengland&timePeriod=2025&baseline=1) crashes for me.
- All constituencies are available in the region selection pane, so if I select one without the beta URL parameter, I'm not sure how this behavior is handled, because...
- Even with the beta URL parameter, the site crashes for me. I believe this may be due to
PoliciesModelledPopup, which you may not be seeing locally due to having cookies enabled, but I believe I'm testing with them disabled. - Looks like tests don't currently run because of a lint failure.
|
Can you provide a preview link that crashes? I can't reproduce so far. |
|
On 3., good point. I'll hide them without the query parameter. |
|
The below link breaks for me: https://policyengine-app-git-nikhilwoodruff-issue2369-policy-engine.vercel.app/uk/policy?focus=policyOutput.policyBreakdown&reform=76756®ion=country%2Fengland&timePeriod=2025&baseline=1 I'm guessing this is at least partly because I clicked to deny cookies when the pop-up came up. In order to reproduce, you might have to empty your own. |
|
Can you try again now, with the API update? |
There was a problem hiding this comment.
Thanks for the back-end changes @nikhilwoodruff, I now was able to get this running. I have a few questions & suggestions and one or two places where I think changes to implementation might be warranted. Looking forward to your thoughts! This is a very cool feature, glad to see this in action.
…-app into nikhilwoodruff/issue2369
There was a problem hiding this comment.
Very cool feature @nikhilwoodruff. Per our discussion, I've flagged one issue that's still occurring, but will open as a separate issue.
| return !!searchParams[param]; | ||
| }; | ||
| const uk_local_areas_beta = hasParam("uk_local_areas_beta") | ||
| ? searchParams.get("uk_local_areas_beta") |
There was a problem hiding this comment.
issue, non-blocking: I believe this will show unintended outputs when the URL param uk_local_areas_beta is given any value
I'm not 100% positive on the causality, partly because getPolicyOutputTree is a quite limiting function that I'd opt to deprecate in front end v2, but here's what happens:
- I, as a user inputting a policy, toggle "Enable UK local areas" to "on", adding "uk_local_areas_beta=true" to the URL
- I then toggle it to "off", updating the URL param to "uk_local...=false"
- If I visit this URL, which contains that param, I can still see the beta output items
I'm going to merge this code, but open this as a separate issue. Flagging here for down the road.
Fixes #2369
Requires PolicyEngine/policyengine-api#2195 to be merged first.
Description
Added UK constituency regions, plus three charts.
Changes
Please describe the changes in detail.
Screenshots
Screen.Recording.2025-02-20.at.17.05.24.mov
Tests
None.