Skip to content

Generic boundary changes #53

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.

Sign up for GitHub

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

Merged
awdem merged 8 commits into from
Aug 6, 2026
Merged

Generic boundary changes #53

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3fe54f4
filter boundary reviews by public_visibility
awdem Jul 23, 2026
7f1de6b
add currency concept to boundary reviews
awdem Jul 23, 2026
44fdedf
add related ballots to boundary changes pipeline
awdem Jul 27, 2026
61f7a85
add quality check for boundary changes csv
awdem Jul 30, 2026
d7218fa
get division related ballots using ballot -> division -> div set FKs
awdem Jul 30, 2026
7128088
filter ballots by current_status
awdem Jul 30, 2026
321f1bd
fix review filtering by currency
awdem Jul 30, 2026
a596705
remove cancelled ballots from query
awdem Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions cdk/queries/addresses_to_boundary_change.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ UNLOAD (
division_slug,
division_name,
division_official_identifier,
division_related_ballots,
boundary_review_id,
division_type,
division_boundary_wkt,
Expand Down Expand Up @@ -43,6 +44,7 @@ UNLOAD (
nd.division_slug AS new_division_slug,
nd.division_name AS new_division_name,
nd.division_official_identifier AS new_division_official_identifier,
nd.division_related_ballots AS related_ballots,
od.consultation_url,
od.legislation_title,
od.effective_date,
Expand Down Expand Up @@ -110,8 +112,9 @@ UNLOAD (
a.organisation_name,
a.organisation_official_name,
a.organisation_gss,
bts.old_division_slug IS NOT NULL AS boundary_same,
nts.old_division_slug IS NOT NULL AS name_same
a.related_ballots AS related_ballots,
bts.old_division_slug IS NOT NULL AS boundary_same,
nts.old_division_slug IS NOT NULL AS name_same
FROM addresses a
LEFT JOIN boundaries_the_same bts
ON a.old_division_slug = bts.old_division_slug
Expand All @@ -128,7 +131,7 @@ UNLOAD (
division_type,
boundary_review_id,
MAP(
ARRAY['division_type', 'old_division_slug', 'old_division_name', 'old_division_official_identifier', 'old_divisionset_pmtiles_url', 'new_division_slug', 'new_division_name', 'new_division_official_identifier', 'new_divisionset_pmtiles_url', 'change_scenario'],
ARRAY['division_type', 'old_division_slug', 'old_division_name', 'old_division_official_identifier', 'old_divisionset_pmtiles_url', 'new_division_slug', 'new_division_name', 'new_division_official_identifier', 'new_divisionset_pmtiles_url', 'related_ballots', 'change_scenario'],
ARRAY[
division_type,
old_division_slug,
Expand All @@ -139,6 +142,7 @@ UNLOAD (
new_division_name,
new_division_official_identifier,
new_divisionset_pmtiles_url,
related_ballots,
CASE
WHEN boundary_same AND name_same THEN 'NO_CHANGE'
WHEN boundary_same AND NOT name_same THEN 'NAME_CHANGED'
Expand Down
36 changes: 35 additions & 1 deletion ...red_components/lambdas/create_boundary_changes_csv/create_current_boundary_reviews_csv.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,22 @@ def export_sql():
JOIN organisations_organisation o ON o.id = obr.organisation_id
JOIN organisations_organisationgeography og ON og.organisation_id = o.id
WHERE
obr.id IN (963, 964)
obr.public_visibility != 'HIDDEN'

@GeoWill GeoWill Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this the right way round?
Would obr.public_visibility = 'MAP' be better?
This might have to be updated to obr.public_visibility IN ( 'MAP', 'NEW_VALUE'), but I guess that's better than having something come through unintentionally?

@awdem awdem Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I sort of assumed that we'd want to include any potential future value that is not HIDDEN, and since we have to manually set the values on the records in EE anyway, we're avoid having a review pulled through before the data baker layer is ready or whatever. But maybe it's better to be explicit?

AND NOT EXISTS (
SELECT
e.election_id
FROM
elections_election e
WHERE

@GeoWill GeoWill Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we want reviews to be taken out of the csv once they're a certain age. This only get's rid of them if there's a new poll.

@awdem awdem Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've updated this filtering in fc851f5

It's now:

AND NOT EXISTS (
    SELECT
        e.election_id
    FROM
        elections_election e
    WHERE
        e.current_status = 'Approved'
        AND e.current IS NOT TRUE
        AND e.organisation_id = o.id
        AND e.poll_open_date >= obr.effective_date
        AND e.poll_open_date <= CURRENT_DATE  - INTERVAL '20 days'
    LIMIT
        1
)

This means we exclude a boundary review if it's organisation has a non-current, approved election with a polling day that is on or after the effective date and the polling day at least 20 days ago.

So it should get rid of them as elections age basically.

e.current_status = 'Approved'
AND NOT e.cancelled
AND e.current IS NOT TRUE
AND e.organisation_id = o.id
AND e.poll_open_date >= obr.effective_date
AND e.poll_open_date <= CURRENT_DATE - INTERVAL '20 days'
LIMIT
1
)
)
SELECT
r.slug,
Expand All @@ -75,6 +90,25 @@ def export_sql():
d.name AS division_name,
d.official_identifier AS division_official_identifier,
st_astext (dgs.geography) AS division_boundary_wkt,
CASE
WHEN ds.id = r.old_divisionset_id THEN NULL
WHEN ds.id = r.new_divisionset_id THEN COALESCE(
(
SELECT
json_agg(e.election_id)::text
FROM
elections_election AS e
WHERE
e.current_status = 'Approved'
AND not e.cancelled
AND e.division_id = d.id
AND d.divisionset_id = r.new_divisionset_id
AND e.poll_open_date = r.effective_date
),
'[]'
)
ELSE NULL
END AS division_related_ballots,
Comment on lines +93 to +111

@awdem awdem Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm ignoring the old divisions here because the relevant ballots will only be made with the new divisions. I'm also unsure if regex matching the division names to the ballots is the best/most efficient way to do this or if there is a better optimized way.

@chris48s chris48s Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm more concerned about accuracy than efficiency here.

To give a real-word example port-talbot is a substring of local.neath-port-talbot.sandfields-east.2022-05-05

Also piddlington-south would be a substring of piddlington-south-east, etc

If we're going to do string matching, we need to be more robust about it
Or, we need to actually use the FK relationship Ballot --> Division --> SubdividedDivision to link ballot and subdivided Division

@GeoWill GeoWill Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So I was wondering about this too.
I was thinking of ways to get round it like doing a substring match with dots: b LIKE '%.' || d.slug || '.%' or maybe d.slug = ANY(string_to_array(b, '.')).

But I think what Chris is suggesting is probably better. There will be issues for elections which don't have a division_id.

@awdem awdem Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So I've re-rewritten this case statement as:

    CASE
        WHEN ds.id = r.old_divisionset_id THEN NULL
        WHEN ds.id = r.new_divisionset_id THEN COALESCE(
            (
                SELECT
                    json_agg(e.election_id)::text
                FROM
                    elections_election AS e
                WHERE
                    e.division_id = d.id
                    AND d.divisionset_id = r.new_divisionset_id
                    AND e.poll_open_date = r.effective_date
            ),
            '[]'
        )
        ELSE NULL
    END AS division_related_ballots,

Doing it like this has also let me remove the review_related_ballots field from the review sub-query. However, there is something I want to check with you both - Will this ever return >1 election_id? Can a division in a divisionset have more than one ballot on the same day?

Sorry if this is an obvious one - I've possibly been thinking too hard about this.

@GeoWill GeoWill Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought the answer was 'no'.
However I didn't actually know that, just 'how I thought it worked'.

Our database doesn't entirely agree, but I think these might be errors. However the query I used probably shows some extra bits you want for the WHERE clause:

  • e.group_type IS NULL
  • NOT e.cancelled
  • e.current_status = 'Approved'

To see if there were any cases where there were multiple ballots for a single division I did this:

SELECT e1.election_id, e1.group_type, e2.election_id, e2.group_type
FROM
    elections_election e1 JOIN elections_election e2
        ON e1.division_id=e2.division_id
            AND e1.poll_open_date = e2.poll_open_date
            AND e1.election_id != e2.election_id
WHERE e1.group_type IS NULL
  AND e2.group_type IS NULL
  AND NOT e1.cancelled
  AND NOT e2.cancelled
  AND e1.current_status = 'Approved'
  AND e2.current_status = 'Approved';

Which I think is right.

With the duplicates manually removed it gives:

local.castle-point.st-georges.2016-05-05,local.castle-point.st-georges.by.2016-05-05
local.castle-point.st-georges.by.2021-05-06,local.castle-point.st-georges.2021-05-06
local.castle-point.boyce.by.2021-05-06,local.castle-point.boyce.2021-05-06

I'm curious why these are all in Castle Point and twice in St. Georges. Have I done something stupid in the query?
These either need tidying up, or are the counter examples you're asking about. Hopefully the former. Or the query isn't right.

@GeoWill GeoWill Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤔 https://candidates.democracyclub.org.uk/media/official_documents/local.castle-point.st-georges.2021-05-06/statement-of-persons-nominated.pdf and https://candidates.democracyclub.org.uk/elections/local.castle-point.st-georges.by.2021-05-06/sopn/

@awdem awdem Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

thanks I've got e.current_status = 'Approved' in the latest query, but I missed NOT e.cancelled. I left out e.group_type IS NULL, because I assumed only ballots have a FK to a division, is that wrong?

Your query looks good to me. Very weird about castle point, maybe one to check with Peter?

@chris48s chris48s Aug 4, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just to put the discussion we had about this yesterday down in text:

A division can have more than one election on the same day
but it has to be a by-election on the same day as a scheduled election
and if there are new boundaries that forces an all-up election, so you can't have that if there has also been a new divisionset

r.boundary_review_id,
CASE
WHEN ds.id = r.old_divisionset_id THEN 'old'
Expand Down
1 change: 1 addition & 0 deletions cdk/shared_components/tables.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@
"division_name": glue.Schema.STRING,
"division_official_identifier": glue.Schema.STRING,
"division_boundary_wkt": glue.Schema.STRING,
"division_related_ballots": glue.Schema.STRING,

@GeoWill GeoWill Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In aggregator api I called the field ballots.
If we keep this then it will need to be updated in the model there.:

},
partition_keys=[
glue.Column(
Expand Down
70 changes: 70 additions & 0 deletions cdk/stacks/current_boundary_changes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
current_boundary_changes
)

current_boundary_changes_csv_quality_check = (
self.make_current_boundary_changes_csv_quality_check()
)

boundary_review_pairs_map = self.make_boundary_review_pairs_map()

make_addresses_to_boundary_change_partitions = (
Expand Down Expand Up @@ -142,6 +146,7 @@ def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
create_current_boundary_changes_csv_task
)
.next(make_current_boundary_changes_partitions)
.next(current_boundary_changes_csv_quality_check)
.next(boundary_review_pairs_map)
.next(make_addresses_to_boundary_change_partitions)
.next(
Expand Down Expand Up @@ -240,6 +245,71 @@ def make_current_boundary_changes_csv_task(self) -> tasks.LambdaInvoke:
),
)

def make_current_boundary_changes_csv_quality_check(
self,
) -> sfn.Chain:
division_ballots_query = tasks.LambdaInvoke(
self,
"Get divison ballots with more than one ballot",
lambda_function=self.athena_query_lambda,
payload=sfn.TaskInput.from_object(
{
"context": {
"table_name": current_boundary_changes.table_name
},
"QueryString": "SELECT DISTINCT division_official_identifier, json_array_length(json_parse(division_related_ballots)) as division_ballot_count FROM {table_name} WHERE divisionset_generation = 'new' AND json_array_length(json_parse(division_related_ballots)) > 1;",
"blocking": True,
}
),
)

# get results of above Athena query
get_division_ballots_query_result = tasks.AthenaGetQueryResults(
self,
"Get divison ballots with more than one ballot results",
query_execution_id="{% $states.input.Payload.queryExecutionId %}",
query_language=sfn.QueryLanguage.JSONATA,
)
# drop the header row
remove_headers = sfn.Pass(
self,
"Drop header from division ballots query results",
parameters={
"divisions_and_counts": sfn.JsonPath.string_at(
"$.ResultSet.Rows[1:]"
),
},
)
count_results = sfn.Pass(
self,
"Count division ballots query results rows",
query_language=sfn.QueryLanguage.JSONATA,
outputs={
"divisions_and_counts_length": "{% $count($states.input.divisions_and_counts) %}",
},
)
# check each division has exactly one ballot
check_results_count = (
sfn.Choice(self, "Check No divisions have more than one ballot")
.when(
sfn.Condition.number_equals("$.divisions_and_counts_length", 0),
sfn.Pass(
self,
"No divisions have more than one ballot!",
),
)
.otherwise(
sfn.Fail(self, "Some divisions have more than one ballot :(")
)
)

return (
division_ballots_query.next(get_division_ballots_query_result)
.next(remove_headers)
.next(count_results)
.next(check_results_count.afterwards())
)

def make_partitions_task(self, table) -> tasks.LambdaInvoke:
return MakePartitionsConstruct(
self,
Expand Down
Loading