Multiple Geographies #331

alexander-griffen · 2023-10-26T09:18:41Z

So far, this PR:

Downloads the new geographies
Alters the Area pages to better handle them

What is required is:

Allowing the map and table on the explore page to handle the two sets of geographies - with a switch?
Making the area page View less inefficient (probably by moving the lookups to be downloaded to the Area model?)

For #256

codecov · 2023-10-26T09:20:29Z

Codecov Report

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

Comparison is base (e2a12cc) 82.50% compared to head (228f87c) 82.82%.

Files	Patch %	Lines
hub/mixins.py	50.00%	7 Missing and 1 partial ⚠️
hub/models.py	80.76%	5 Missing ⚠️
hub/management/commands/base_importers.py	60.00%	4 Missing ⚠️

Additional details and impacted files

@@                      Coverage Diff                      @@
##           multiple-geometry-staging     #331      +/-   ##
=============================================================
+ Coverage                      82.50%   82.82%   +0.32%     
=============================================================
  Files                             87       90       +3     
  Lines                           2669     2725      +56     
  Branches                         275      278       +3     
=============================================================
+ Hits                            2202     2257      +55     
  Misses                           404      404              
- Partials                          63       64       +1

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

Fill the existing constituencies with the correct AreaType (2010 parliamentary constituencies)

zarino

@alexander-griffen I‘ve cherry-picked the five useful commits out of your original branch and onto main, so the PR diff looks much cleaner now! (Your original branch has been backed up to 256-multiple-geographies-backup in case we ever need it again.)

The "This is a 2010 constituency" card at the top of the Area page looks fine.

I’ll sort out a constituency generation switcher on the Explore page next week.

In the meantime, I‘ve left a few questions / comments inline.

hub/management/commands/import_new_constituencies.py

zarino · 2023-10-27T15:20:27Z

hub/management/commands/import_new_constituencies.py

+            print("Importing Areas")
+        df = pd.read_csv(self.data_file)
+        area_type, created = AreaType.objects.get_or_create(
+            name="2023 Parliamentary Constituency",


It looks like we’ve made a decision, elsewhere (eg: https://pages.mysociety.org/2025-constituencies/) to call these "2025" constituencies. So should we use 2025 Parliamentary Constituency and WMC25 here? /cc @ajparsons

FWIW I discussed this with @ajparsons and we think we should refer to these new constituencies as either "2025 constituencies" or "Future constituencies" (if we want to hedge our bets) when we’re limited for space, and "Constituencies at the next election" if we have more space.

I see @struan has already modified import_new_constituencies.py to use the latter 👍

zarino · 2023-10-27T15:22:50Z

hub/models.py

@@ -388,6 +388,17 @@ def shader_value(self, area):
        return None, None, None


+class AreaType(models.Model):


PS @alexander-griffen I learned a few weeks ago that you can pass a string to models.ForeignKey() when you want to refer to a model that’s defined further down in the file. So, in this case, a line like…

area_type = models.ForeignKey("AreaType", on_delete=models.CASCADE, null=True)

…would have saved you having to shuffle this Class further up the file.

zarino · 2023-10-27T15:25:22Z

hub/templates/hub/area.html

@@ -39,10 +39,40 @@ <h1 class="m-0">{{ area.name }}</h1>
                </div>
            </div>
            <div class="col-lg-9 offset-xl-1">
-
+            {% if area_type == "WMC" %}
+            <div class="card flex-grow-1 dataset-card">


It’d be good to have one of these cards on the WMC23 pages too, but going the other way (ie: saying "This is a 2025 (2023?) constituency … it replaced the following 2010 constituency/ies: …"

I'm going to fix this as part of #337

hub/templates/hub/area.html

struan

Mostly looks fine. one comment about the geometry stuff which is hampered by lack of data.

hub/management/commands/import_new_constituencies.py

struan · 2023-11-06T16:39:45Z

A further issue is that GSS codes turn out not be unique across the two different boundary sets so we need to update import scripts etc to distinguish areas based on GSS/Name and area type.

I am going to add some methods to the Area model for looking up areas so we can consolidate this in once place.

zarino

@struan @alexander-griffen I’ve added a commit that fixes a few of the comments I left last week. The other open comments are all beyond my ability.

Generally, this seems ok. It’d be nice to get those outstanding comments addressed though.

zarino

@struan I think this is all looking good so far. Obviously we’re missing all the WMC23 datasets, so the Area page for a WMC23 constituency looks particularly empty/broken. Do you want to implement updated import scripts as part of this PR, or create a separate ticket and PR for it?

Note that this does currently break the table and map

This is to ensure urls work as expected

mostly to default the area_type

- Only show MP section and nav link on WMC pages (ie: not WMC23). - Fix messed up indentation on "This is a 2010 constituency" info box.

alexander-griffen requested a review from zarino October 26, 2023 09:18

alexander-griffen added 2 commits October 27, 2023 14:56

Migration: Add area_type field to DataType model

d9cc41a

MIGRATION: Fill the area_type values

7551206

Fill the existing constituencies with the correct AreaType (2010 parliamentary constituencies)

zarino force-pushed the 256-multiple-geographies branch from af1a72c to 5565933 Compare October 27, 2023 14:27

zarino reviewed Oct 27, 2023

View reviewed changes

struan self-requested a review October 30, 2023 11:57

struan requested changes Oct 31, 2023

View reviewed changes

hub/management/commands/import_new_constituencies.py Outdated Show resolved Hide resolved

hub/management/commands/import_new_constituencies.py Outdated Show resolved Hide resolved

struan requested a review from zarino November 9, 2023 16:23

struan changed the base branch from main to multiple-geometry-staging November 9, 2023 16:23

zarino reviewed Nov 13, 2023

View reviewed changes

struan requested a review from zarino November 13, 2023 17:15

zarino approved these changes Nov 14, 2023

View reviewed changes

alexander-griffen and others added 14 commits November 21, 2023 14:49

IMPORT: 2023 constituency data

7b93f5e

Note that this does currently break the table and map

Add __str__ method to AreaType

98d690b

This is to ensure urls work as expected

WIP: Add info based on AreaType

b5edfa2

Markup for area_type switcher on Explore page

16e8ba9

update explore map data to filter by constituency type

ba19d43

update explore table to filter by constituency type

7e5a1e8

update map code to handle changing initial consitituency types

cf912b3

add area getting methods to area class

dc4db6f

mostly to default the area_type

update base importers to use area_type as part of area look up

d88ee17

update import script for non unique GSS codes

68461c6

add filtering by area type to admin

1976f39

update area tests for non unique gss codes

0274abe

update area model to be unique on gss and area type

9dc0bf3

Area page tweaks

228f87c

- Only show MP section and nav link on WMC pages (ie: not WMC23). - Fix messed up indentation on "This is a 2010 constituency" info box.

struan force-pushed the 256-multiple-geographies branch from 863476e to 228f87c Compare November 21, 2023 14:50

struan merged commit 228f87c into multiple-geometry-staging Nov 21, 2023
6 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple Geographies #331

Multiple Geographies #331

alexander-griffen commented Oct 26, 2023 •

edited by struan

Loading

codecov bot commented Oct 26, 2023 •

edited

Loading

zarino left a comment

zarino Oct 27, 2023

zarino Nov 14, 2023

zarino Oct 27, 2023

zarino Oct 27, 2023

struan Nov 13, 2023

struan left a comment

struan commented Nov 6, 2023

zarino left a comment

zarino left a comment

		@@ -388,6 +388,17 @@ def shader_value(self, area):
		return None, None, None


		class AreaType(models.Model):

Multiple Geographies #331

Multiple Geographies #331

Conversation

alexander-griffen commented Oct 26, 2023 • edited by struan Loading

codecov bot commented Oct 26, 2023 • edited Loading

Codecov Report

zarino left a comment

Choose a reason for hiding this comment

zarino Oct 27, 2023

Choose a reason for hiding this comment

zarino Nov 14, 2023

Choose a reason for hiding this comment

zarino Oct 27, 2023

Choose a reason for hiding this comment

zarino Oct 27, 2023

Choose a reason for hiding this comment

struan Nov 13, 2023

Choose a reason for hiding this comment

struan left a comment

Choose a reason for hiding this comment

struan commented Nov 6, 2023

zarino left a comment

Choose a reason for hiding this comment

zarino left a comment

Choose a reason for hiding this comment

alexander-griffen commented Oct 26, 2023 •

edited by struan

Loading

codecov bot commented Oct 26, 2023 •

edited

Loading