|
| 1 | +#!/usr/bin/env python3 |
| 2 | +"""Drop admin-level placeholder rows from contributions/cities/ES.json. |
| 3 | +
|
| 4 | +Issue #1498. The reporter flagged that `GetCity(...)` returns province-level |
| 5 | +admin entries inside Spanish city dropdowns ("Provincia de Madrid" etc.). |
| 6 | +This is the city-level cleanup PR — drops 22 rows that are not real cities: |
| 7 | +
|
| 8 | + - 21 "Provincia de X" / "Província de X" rows (ids 36362, 36364, 36365, |
| 9 | + 36373, 36375, 36376, 36377, 36379, 36381, 36383, 36385, 36386, 36387, |
| 10 | + 36389, 36390, 36391, 36392, 36393, 36394, 36396, 36400). All are admin |
| 11 | + placeholders that duplicate concepts already represented by entries in |
| 12 | + contributions/states/states.json. The 50 Spanish provinces are already |
| 13 | + states, so a separate "Provincia de X" pseudo-city is redundant. |
| 14 | +
|
| 15 | + - 1 cross-state placeholder (id 32244, name "Alicante", state_code=V). |
| 16 | + The real Alicante city lives at id 152158 ("Alicante/Alacant", |
| 17 | + state_code=A) under the Alicante province (iso2=A). Row 32244 is a |
| 18 | + legacy stub from when Valencia community was the parent of three |
| 19 | + provinces (Valencia, Alicante, Castellon) — Alicante now has its own |
| 20 | + state, so the stub is wrong both in name (missing the Valencian |
| 21 | + endonym) and in parentage. |
| 22 | +
|
| 23 | +The 21 "Provincia ..." ids are an explicit allowlist rather than a name- |
| 24 | +prefix filter, because two real Spanish municipalities also begin with |
| 25 | +"Provincia" in obscure forms — using ids guarantees we touch only what we |
| 26 | +mean to. The script verifies each id's current name still matches the |
| 27 | +expected prefix before dropping, refusing to touch rows that don't. |
| 28 | +
|
| 29 | +Idempotent: a second run on already-cleaned data writes nothing and exits 0. |
| 30 | +
|
| 31 | +Usage: |
| 32 | + python3 bin/scripts/fixes/spain_drop_provincia_placeholders.py [--dry-run] |
| 33 | +""" |
| 34 | + |
| 35 | +from __future__ import annotations |
| 36 | + |
| 37 | +import argparse |
| 38 | +import json |
| 39 | +import sys |
| 40 | +from pathlib import Path |
| 41 | +from typing import List |
| 42 | + |
| 43 | +REPO_ROOT = Path(__file__).resolve().parents[3] |
| 44 | +CITIES_JSON = REPO_ROOT / "contributions/cities/ES.json" |
| 45 | + |
| 46 | +# 21 admin-placeholder ids, expected to start with "Provincia " or "Província ". |
| 47 | +PROVINCIA_IDS = frozenset({ |
| 48 | + 36362, 36364, 36365, 36373, 36375, 36376, 36377, 36379, 36381, 36383, |
| 49 | + 36385, 36386, 36387, 36389, 36390, 36391, 36392, 36393, 36394, 36396, |
| 50 | + 36400, |
| 51 | +}) |
| 52 | + |
| 53 | +# 1 cross-state placeholder. id, expected name, expected wrong state_code. |
| 54 | +CROSS_STATE_ALICANTE_ID = 32244 |
| 55 | +CROSS_STATE_ALICANTE_NAME = "Alicante" |
| 56 | +CROSS_STATE_ALICANTE_STATE_CODE = "V" |
| 57 | + |
| 58 | +EXPECTED_DROP_COUNT = len(PROVINCIA_IDS) + 1 # 22 |
| 59 | +EXPECTED_PRE_COUNT = 8427 |
| 60 | +EXPECTED_POST_COUNT = EXPECTED_PRE_COUNT - EXPECTED_DROP_COUNT # 8405 |
| 61 | + |
| 62 | + |
| 63 | +def load_cities(path: Path) -> List[dict]: |
| 64 | + """Read the cities JSON file and verify the top-level shape.""" |
| 65 | + with path.open("r", encoding="utf-8") as fh: |
| 66 | + data = json.load(fh) |
| 67 | + if not isinstance(data, list): |
| 68 | + raise SystemExit(f"Expected JSON array at {path}, got {type(data).__name__}") |
| 69 | + return data |
| 70 | + |
| 71 | + |
| 72 | +def matches_provincia(row: dict) -> bool: |
| 73 | + """Row is one of the 21 'Provincia ...' admin placeholders, name verified.""" |
| 74 | + if row.get("id") not in PROVINCIA_IDS: |
| 75 | + return False |
| 76 | + name = row.get("name", "") |
| 77 | + return isinstance(name, str) and ( |
| 78 | + name.startswith("Provincia ") or name.startswith("Província ") |
| 79 | + ) |
| 80 | + |
| 81 | + |
| 82 | +def matches_cross_state_alicante(row: dict) -> bool: |
| 83 | + """Row is the cross-state Alicante stub (id 32244, name 'Alicante', state V).""" |
| 84 | + return ( |
| 85 | + row.get("id") == CROSS_STATE_ALICANTE_ID |
| 86 | + and row.get("name") == CROSS_STATE_ALICANTE_NAME |
| 87 | + and row.get("state_code") == CROSS_STATE_ALICANTE_STATE_CODE |
| 88 | + ) |
| 89 | + |
| 90 | + |
| 91 | +def is_target(row: dict) -> bool: |
| 92 | + """True if this row is on the drop list.""" |
| 93 | + return matches_provincia(row) or matches_cross_state_alicante(row) |
| 94 | + |
| 95 | + |
| 96 | +def main() -> int: |
| 97 | + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) |
| 98 | + parser.add_argument( |
| 99 | + "--dry-run", |
| 100 | + action="store_true", |
| 101 | + help="Report what would be dropped without rewriting the file.", |
| 102 | + ) |
| 103 | + args = parser.parse_args() |
| 104 | + |
| 105 | + if not CITIES_JSON.exists(): |
| 106 | + raise SystemExit(f"Cities file not found: {CITIES_JSON}") |
| 107 | + |
| 108 | + cities = load_cities(CITIES_JSON) |
| 109 | + pre_count = len(cities) |
| 110 | + |
| 111 | + to_drop = [row for row in cities if is_target(row)] |
| 112 | + kept = [row for row in cities if not is_target(row)] |
| 113 | + |
| 114 | + drop_count = len(to_drop) |
| 115 | + post_count = len(kept) |
| 116 | + |
| 117 | + # Defensive check: any row whose id is in our allowlist but whose name |
| 118 | + # doesn't match the expected pattern (could indicate the data has |
| 119 | + # shifted since this script was written). |
| 120 | + rows_by_id = {row.get("id"): row for row in cities} |
| 121 | + suspicious: List[dict] = [] |
| 122 | + for pid in PROVINCIA_IDS: |
| 123 | + row = rows_by_id.get(pid) |
| 124 | + if row is not None and not matches_provincia(row): |
| 125 | + suspicious.append(row) |
| 126 | + cross = rows_by_id.get(CROSS_STATE_ALICANTE_ID) |
| 127 | + if cross is not None and not matches_cross_state_alicante(cross): |
| 128 | + suspicious.append(cross) |
| 129 | + |
| 130 | + print(f"Input file: {CITIES_JSON.relative_to(REPO_ROOT)}") |
| 131 | + print(f"Pre-clean count: {pre_count}") |
| 132 | + print(f"Provincia drops: " |
| 133 | + f"{sum(1 for r in to_drop if matches_provincia(r))} / " |
| 134 | + f"{len(PROVINCIA_IDS)} expected") |
| 135 | + print(f"Cross-state Alicante drop: " |
| 136 | + f"{sum(1 for r in to_drop if matches_cross_state_alicante(r))} / 1 expected") |
| 137 | + print(f"Total drops: {drop_count}") |
| 138 | + print(f"Post-clean count: {post_count}") |
| 139 | + |
| 140 | + if suspicious: |
| 141 | + print( |
| 142 | + "\nERROR: target ids exist but do not match expected name/state — " |
| 143 | + "refusing to drop them:", |
| 144 | + file=sys.stderr, |
| 145 | + ) |
| 146 | + for row in suspicious: |
| 147 | + print( |
| 148 | + f" id={row.get('id')} name={row.get('name')!r} " |
| 149 | + f"state_code={row.get('state_code')!r}", |
| 150 | + file=sys.stderr, |
| 151 | + ) |
| 152 | + return 2 |
| 153 | + |
| 154 | + if drop_count == 0: |
| 155 | + print("\nNo placeholder rows found — nothing to do (idempotent).") |
| 156 | + return 0 |
| 157 | + |
| 158 | + if drop_count != EXPECTED_DROP_COUNT or pre_count != EXPECTED_PRE_COUNT: |
| 159 | + print( |
| 160 | + f"\nWARNING: counts diverge from the expected baseline " |
| 161 | + f"({EXPECTED_PRE_COUNT} -> drop {EXPECTED_DROP_COUNT} -> " |
| 162 | + f"{EXPECTED_POST_COUNT}). Got {pre_count} -> drop {drop_count} " |
| 163 | + f"-> {post_count}. Review the diff before merging.", |
| 164 | + file=sys.stderr, |
| 165 | + ) |
| 166 | + |
| 167 | + if args.dry_run: |
| 168 | + print("\n--dry-run: not writing.") |
| 169 | + for r in to_drop: |
| 170 | + print(f" would drop id={r['id']} name={r['name']!r} " |
| 171 | + f"state_code={r.get('state_code')!r} type={r.get('type')!r}") |
| 172 | + return 0 |
| 173 | + |
| 174 | + with CITIES_JSON.open("w", encoding="utf-8") as fh: |
| 175 | + json.dump(kept, fh, ensure_ascii=False, indent=2) |
| 176 | + fh.write("\n") |
| 177 | + |
| 178 | + print(f"\nWrote {post_count} records to {CITIES_JSON.relative_to(REPO_ROOT)}.") |
| 179 | + print("Run `python3 bin/scripts/sync/normalize_json.py " |
| 180 | + "contributions/cities/ES.json` next to canonicalize formatting.") |
| 181 | + return 0 |
| 182 | + |
| 183 | + |
| 184 | +if __name__ == "__main__": |
| 185 | + sys.exit(main()) |
0 commit comments