-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add UK Parliamentary constituency estimates #2195
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @nikhilwoodruff, thanks for all of this. TL;DR:
- I believe Northern Irish and Welsh runs would crash with these changes
- I've flagged a few places I think typing and/or Pydantic schematization/dataclasses would be beneficial
- I think we should really discuss codifying "primary," "secondary," and "tertiary" country-level entities throughout the system, as I think this code illustrates the limits of the single-tier "region" designation
Per discussions with Nikhil, we'll be handling the sub-national entity standardization question in a separate issue down the road, which I'll be opening as an issue in |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2195 +/- ##
==========================================
- Coverage 75.48% 74.49% -0.99%
==========================================
Files 78 78
Lines 2921 2992 +71
Branches 309 322 +13
==========================================
+ Hits 2205 2229 +24
- Misses 651 698 +47
Partials 65 65 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nikhilwoodruff! I'll move onto the corresponding front-end PR now. Just had a question meant to double-check something, as well as a couple minor comments.
Fixes #2191