Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import overlapping constituencies #345

Merged
merged 3 commits into from
Nov 23, 2023

Conversation

struan
Copy link
Member

@struan struan commented Nov 14, 2023

Fixes #337

Copy link

codecov bot commented Nov 14, 2023

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (50570ce) 82.93% compared to head (18dd032) 82.45%.

❗ Current head 18dd032 differs from pull request most recent head 34bff86. Consider uploading reports for the commit 34bff86 to get more accurate results

Files Patch % Lines
hub/views/area.py 20.00% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@                      Coverage Diff                      @@
##           multiple-geometry-staging     #345      +/-   ##
=============================================================
- Coverage                      82.93%   82.45%   -0.48%     
=============================================================
  Files                             92       93       +1     
  Lines                           2742     2753      +11     
  Branches                         279      281       +2     
=============================================================
- Hits                            2274     2270       -4     
- Misses                           404      418      +14     
- Partials                          64       65       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@struan struan requested a review from zarino November 21, 2023 10:56
@struan struan force-pushed the 256-multiple-geographies branch from 863476e to 228f87c Compare November 21, 2023 14:50
@struan struan force-pushed the 337-store-cons-overlaps branch from 6c453f2 to b61f15d Compare November 21, 2023 15:24
@struan struan changed the base branch from 256-multiple-geographies to multiple-geometry-staging November 21, 2023 15:27
@struan
Copy link
Member Author

struan commented Nov 21, 2023

@zarino this is now ready to review

Copy link
Member

@zarino zarino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me @struan. Can confirm the migration and import script worked for me in the Docker container, and things look right in the UI.

I’ve added a fixup commit that changes the markup a little in area.html – with fresh eyes, I was annoyed by how the table layout tended to break each constituency name onto two lines, and the text-muted paragraph after each constituency still felt too prominent. So I’ve gone with a grid layout now, which is optimised for 2–3 replacement constituencies but will handle more or fewer than that too. I’ve also tweaked the wording in the special case where a constituency is a straight 1-for-1 swap, now saying that the constituency is "replaced" rather than being "divided".

Screenshot 2023-11-21 at 18 13 24 Screenshot 2023-11-21 at 18 13 31 Screenshot 2023-11-21 at 18 13 38 Screenshot 2023-11-21 at 18 13 44

@struan struan force-pushed the 337-store-cons-overlaps branch from 18dd032 to 34bff86 Compare November 23, 2023 10:38
@struan struan merged commit 34bff86 into multiple-geometry-staging Nov 23, 2023
5 checks passed
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.

2 participants