Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
c4b7868cbf |
No files matched your search
+12
-5
@@ -20,6 +20,18 @@ A locality whose outcodes hold fewer than MIN_SCHOOLS schools is not
|
||||
published, so a typo produces no page rather than an empty one. Places that
|
||||
fail that check are logged at startup, because a locality you meant to publish
|
||||
quietly not appearing is the failure worth hearing about.
|
||||
|
||||
Two rules for anything added here.
|
||||
|
||||
**Sub-borough districts only.** A London borough is a local authority and
|
||||
already has a page at /schools/authority/[la] covering all of its schools; a
|
||||
locality defined by two or three outcodes would be a partial, near-duplicate
|
||||
subset of it. Hackney, Islington, Greenwich and Ealing were all in the first
|
||||
draft for that reason and have been removed.
|
||||
|
||||
**The slug must not match a GIAS town.** "Richmond" did — GIAS has a Richmond
|
||||
in North Yorkshire with 37 schools — so the London one could never publish.
|
||||
The registry skips any locality that collides and logs it.
|
||||
"""
|
||||
|
||||
# slug -> (display name, outcodes)
|
||||
@@ -30,16 +42,11 @@ LOCALITY_OUTCODES: dict[str, tuple[str, tuple[str, ...]]] = {
|
||||
"shoreditch": ("Shoreditch", ("EC2A", "E1")),
|
||||
"peckham": ("Peckham", ("SE15",)),
|
||||
"brixton": ("Brixton", ("SW2", "SW9")),
|
||||
"hackney": ("Hackney", ("E5", "E8", "E9")),
|
||||
"islington": ("Islington", ("N1", "N5", "N7")),
|
||||
"camden-town": ("Camden Town", ("NW1",)),
|
||||
"greenwich": ("Greenwich", ("SE10",)),
|
||||
"wimbledon": ("Wimbledon", ("SW19",)),
|
||||
"putney": ("Putney", ("SW15",)),
|
||||
"fulham": ("Fulham", ("SW6",)),
|
||||
"chiswick": ("Chiswick", ("W4",)),
|
||||
"ealing": ("Ealing", ("W5", "W13")),
|
||||
"richmond": ("Richmond", ("TW9", "TW10")),
|
||||
"stratford": ("Stratford", ("E15",)),
|
||||
"walthamstow": ("Walthamstow", ("E17",)),
|
||||
"tooting": ("Tooting", ("SW17",)),
|
||||
|
||||
+13
-4
@@ -128,10 +128,19 @@ def _locality_places(df, publishable: set[int],
|
||||
out: dict[str, Place] = {}
|
||||
for slug, (name, outcodes) in LOCALITY_OUTCODES.items():
|
||||
if slug in town_slugs:
|
||||
raise ValueError(
|
||||
f"locality {slug!r} collides with a published town of the same "
|
||||
"slug; publishing both would shadow the town silently"
|
||||
)
|
||||
# Skip, do not raise. The guard exists so a locality never
|
||||
# silently shadows a town — skipping achieves that, and the error
|
||||
# log makes it loud.
|
||||
#
|
||||
# Raising here took down sitemap generation for all 25,000 school
|
||||
# pages when "richmond" met the GIAS town Richmond in North
|
||||
# Yorkshire. Worse, GIAS town names change without any code change,
|
||||
# so a raise means curated data can break the site spontaneously.
|
||||
# A curation mistake must cost one page, not the sitemap.
|
||||
logger.error(
|
||||
"locality %r collides with the published town of the same "
|
||||
"slug and has been skipped; rename it or remove it", slug)
|
||||
continue
|
||||
group = working[working["_oc"].isin(outcodes)]
|
||||
urns = tuple(sorted({int(u) for u in group["urn"]} & publishable))
|
||||
if len(urns) < MIN_SCHOOLS:
|
||||
|
||||
@@ -108,16 +108,42 @@ def test_locality_below_the_threshold_is_not_published(monkeypatch):
|
||||
assert "locality:nowhere" not in reg
|
||||
|
||||
|
||||
def test_a_locality_may_not_shadow_a_viable_town(monkeypatch):
|
||||
# Silently shadowing a town would lose a page carrying real demand.
|
||||
def test_a_locality_may_not_shadow_a_viable_town(monkeypatch, caplog):
|
||||
"""A colliding locality is skipped loudly, and the town survives.
|
||||
|
||||
This used to raise, which took down sitemap generation for all 25,000
|
||||
school pages the first time a curated slug met a real GIAS town. Curated
|
||||
data must not be able to break the site — and GIAS town names change with
|
||||
no code change at all, so the raise could fire spontaneously.
|
||||
"""
|
||||
import logging
|
||||
|
||||
from backend import localities
|
||||
monkeypatch.setattr(localities, "LOCALITY_OUTCODES",
|
||||
{"brentwood": ("Brentwood", ("CM13",))})
|
||||
rows = _town(MIN_SCHOOLS, "Brentwood", "Essex")
|
||||
for r in rows:
|
||||
r["postcode"] = "CM13 1AA"
|
||||
with pytest.raises(ValueError, match="brentwood"):
|
||||
build_place_registry(_df(rows))
|
||||
|
||||
with caplog.at_level(logging.ERROR):
|
||||
reg = build_place_registry(_df(rows))
|
||||
|
||||
assert "locality:brentwood" not in reg # skipped
|
||||
assert "town:brentwood" in reg # the town is untouched
|
||||
assert "brentwood" in caplog.text # and it was loud about it
|
||||
|
||||
|
||||
def test_a_locality_collision_does_not_break_the_rest_of_the_registry(monkeypatch):
|
||||
# The whole point of skipping rather than raising.
|
||||
from backend import localities
|
||||
monkeypatch.setattr(localities, "LOCALITY_OUTCODES",
|
||||
{"brentwood": ("Brentwood", ("CM13",))})
|
||||
rows = _town(MIN_SCHOOLS, "Brentwood", "Essex")
|
||||
for r in rows:
|
||||
r["postcode"] = "CM13 1AA"
|
||||
reg = build_place_registry(_df(rows))
|
||||
assert "authority:essex" in reg
|
||||
assert "outcode:cm13" in reg
|
||||
|
||||
|
||||
def test_outcode_places_are_built_from_postcodes():
|
||||
@@ -174,3 +200,32 @@ def test_the_pipeline_seed_mirrors_the_canonical_module():
|
||||
for row in csv.DictReader(seed_path.open())
|
||||
}
|
||||
assert seed == LOCALITY_OUTCODES
|
||||
|
||||
|
||||
def test_no_curated_locality_names_a_london_borough():
|
||||
"""Boroughs are authorities and already have a page.
|
||||
|
||||
A locality defined by two or three outcodes inside a borough would be a
|
||||
partial, near-duplicate subset of that authority page — the exact
|
||||
thin-content failure the two-namespace design exists to avoid. Hackney,
|
||||
Islington, Greenwich and Ealing were all in the first draft.
|
||||
|
||||
Hardcoded rather than read from the corpus because this must fail in CI,
|
||||
where there is no database.
|
||||
"""
|
||||
from backend.localities import LOCALITY_OUTCODES
|
||||
|
||||
boroughs = {
|
||||
"barking-and-dagenham", "barnet", "bexley", "brent", "bromley",
|
||||
"camden", "croydon", "ealing", "enfield", "greenwich", "hackney",
|
||||
"hammersmith-and-fulham", "haringey", "harrow", "havering",
|
||||
"hillingdon", "hounslow", "islington", "kensington-and-chelsea",
|
||||
"kingston-upon-thames", "lambeth", "lewisham", "merton", "newham",
|
||||
"redbridge", "richmond-upon-thames", "southwark", "sutton",
|
||||
"tower-hamlets", "waltham-forest", "wandsworth", "westminster",
|
||||
}
|
||||
named = boroughs & set(LOCALITY_OUTCODES)
|
||||
assert not named, (
|
||||
f"these are boroughs, not districts: {sorted(named)} - they already "
|
||||
"have an authority page covering every school"
|
||||
)
|
||||
@@ -5,16 +5,11 @@ clapham,Clapham,SW4,London
|
||||
shoreditch,Shoreditch,EC2A|E1,London
|
||||
peckham,Peckham,SE15,London
|
||||
brixton,Brixton,SW2|SW9,London
|
||||
hackney,Hackney,E5|E8|E9,London
|
||||
islington,Islington,N1|N5|N7,London
|
||||
camden-town,Camden Town,NW1,London
|
||||
greenwich,Greenwich,SE10,London
|
||||
wimbledon,Wimbledon,SW19,London
|
||||
putney,Putney,SW15,London
|
||||
fulham,Fulham,SW6,London
|
||||
chiswick,Chiswick,W4,London
|
||||
ealing,Ealing,W5|W13,London
|
||||
richmond,Richmond,TW9|TW10,London
|
||||
stratford,Stratford,E15,London
|
||||
walthamstow,Walthamstow,E17,London
|
||||
tooting,Tooting,SW17,London
|
||||
|
||||
|
Reference in new issue
Block a user