Регион точки резолвится по настоящей границе, а не по прямоугольнику (#3052) #3522
5 changed files with 220 additions and 33 deletions
File diff suppressed because one or more lines are too long
|
|
@ -10,13 +10,21 @@
|
|||
Регион 50 (Московская область) отложен сознательно — обоснование в #2996:
|
||||
10 121 текстовое имя города против 612 у Москвы, вся мина имён — в области.
|
||||
|
||||
#3052: `region_for_point` резолвит по НАСТОЯЩЕЙ границе региона (полигон
|
||||
OSM/Nominatim, `region_boundaries/boundaries.geojson.json`), не по
|
||||
прямоугольнику `bbox_region` — прямоугольники Москвы и области пересекались
|
||||
(Химки/Реутов/Котельники/Люберцы уходили в Москву), полигоны нет.
|
||||
|
||||
Модуль — ЛИСТ дерева импортов: не импортирует ничего из app.* (его читают
|
||||
geocoder / location_index / matching.normalize, циклы недопустимы).
|
||||
geocoder / location_index / matching.normalize, циклы недопустимы). json/
|
||||
pathlib — стандартная библиотека, листовость не нарушают.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from dataclasses import dataclass
|
||||
from pathlib import Path
|
||||
|
||||
# bbox = (lat_min, lat_max, lon_min, lon_max) — тот же порядок, что исторический
|
||||
# geocoder.EKB_BBOX_TIGHT (см. is_within_bbox ниже).
|
||||
|
|
@ -90,6 +98,80 @@ def is_within_bbox(lat: float, lon: float, bbox: BBox) -> bool:
|
|||
return lat_min <= lat <= lat_max and lon_min <= lon <= lon_max
|
||||
|
||||
|
||||
# ── #3052: настоящая граница региона (point-in-polygon вместо bbox) ─────────
|
||||
#
|
||||
# Источник — region_boundaries/boundaries.geojson.json: полигоны OSM/Nominatim
|
||||
# (см. поля _source/_license внутри файла, лицензия ODbL требует их хранить).
|
||||
# Парсим один раз на импорте модуля — 27 точек лишний JSON-parse на каждый
|
||||
# вызов region_for_point был бы дороже самого ray casting.
|
||||
#
|
||||
# Ring — одно кольцо GeoJSON (lon, lat) точек: внешний контур ИЛИ дыра.
|
||||
# Полигон/мультиполигон региона хранится как ПЛОСКИЙ список всех его колец
|
||||
# (для MultiPolygon 77 — кольца всех 10 частей вперемешку, у Polygon 50 —
|
||||
# внешнее кольцо + 9 дыр). Плоский список работает благодаря even-odd
|
||||
# правилу: точка внутри региона ⟺ она попадает внутрь НЕЧЁТНОГО числа колец
|
||||
# из списка. Это ОДНОВРЕМЕННО корректно обрабатывает дыры (Москва — дыра в
|
||||
# кольцах региона 50: попадание в кольцо-дыру снимает чётность, снятую
|
||||
# внешним кольцом) и непересекающиеся части мультиполигона (попадание ровно
|
||||
# в одно кольцо — нечётность не портится соседними частями, которые точка не
|
||||
# задевает) — без явного разделения "внешний контур минус дыры".
|
||||
Ring = list[tuple[float, float]]
|
||||
|
||||
_BOUNDARIES_PATH = Path(__file__).parent / "region_boundaries" / "boundaries.geojson.json"
|
||||
|
||||
|
||||
def _flatten_rings(geometry: dict[str, object]) -> list[Ring]:
|
||||
"""Все кольца геометрии (Polygon или MultiPolygon) одним плоским списком."""
|
||||
coords = geometry["coordinates"]
|
||||
if geometry["type"] == "Polygon":
|
||||
polygons = [coords]
|
||||
elif geometry["type"] == "MultiPolygon":
|
||||
polygons = coords # type: ignore[assignment]
|
||||
else:
|
||||
raise ValueError(f"неподдержанный тип геометрии: {geometry['type']}")
|
||||
rings: list[Ring] = []
|
||||
for polygon in polygons:
|
||||
for ring in polygon: # type: ignore[union-attr]
|
||||
rings.append([(pt[0], pt[1]) for pt in ring]) # type: ignore[index]
|
||||
return rings
|
||||
|
||||
|
||||
def _load_region_boundaries() -> dict[int, list[Ring]]:
|
||||
"""code региона → плоский список колец его полигона/мультиполигона."""
|
||||
with _BOUNDARIES_PATH.open(encoding="utf-8") as f:
|
||||
payload = json.load(f)
|
||||
return {
|
||||
int(code): _flatten_rings(entry["geometry"]) for code, entry in payload["regions"].items()
|
||||
}
|
||||
|
||||
|
||||
_REGION_BOUNDARIES: dict[int, list[Ring]] = _load_region_boundaries()
|
||||
|
||||
|
||||
def _point_in_ring(x: float, y: float, ring: Ring) -> bool:
|
||||
"""PNPOLY (W. R. Franklin) ray casting для одного кольца."""
|
||||
inside = False
|
||||
xj, yj = ring[-1]
|
||||
for xi, yi in ring:
|
||||
if (yi > y) != (yj > y) and x < (xj - xi) * (y - yi) / (yj - yi) + xi:
|
||||
inside = not inside
|
||||
xj, yj = xi, yi
|
||||
return inside
|
||||
|
||||
|
||||
def _point_in_region_polygon(code: int, lat: float, lon: float) -> bool:
|
||||
"""True если (lat, lon) внутри настоящей границы региона `code`.
|
||||
|
||||
even-odd по всем кольцам сразу (см. комментарий выше про плоский список).
|
||||
Региона без загруженной границы — граничит только по bbox (см. вызов в
|
||||
region_for_point), сюда такой код не попадает."""
|
||||
inside = False
|
||||
for ring in _REGION_BOUNDARIES[code]:
|
||||
if _point_in_ring(lon, lat, ring):
|
||||
inside = not inside
|
||||
return inside
|
||||
|
||||
|
||||
# Тиры обогащения (строковые ключи — по label'ам _with_budget в estimator).
|
||||
TIER_AVITO_IMV = "avito_imv"
|
||||
TIER_YANDEX_VALUATION = "yandex_valuation"
|
||||
|
|
@ -262,35 +344,55 @@ def _bbox_area(bbox: BBox) -> float:
|
|||
# Порядок обхода для region_for_point: от САМОГО специфичного (маленький
|
||||
# bbox_region) к самому общему — НЕ sorted(REGIONS) по числовому коду.
|
||||
#
|
||||
# Почему код региона как ключ порядка сломался: bbox_region(50) (Московская
|
||||
# область целиком, lat 54.20..56.96/lon 35.14..40.21) геометрически СОДЕРЖИТ
|
||||
# bbox_region(77) (Москва, 55.10..56.10/36.80..38.10) как прямоугольники — а
|
||||
# 50 < 66 < 77 по числу. При обходе `sorted(REGIONS)` регион 50 проверялся бы
|
||||
# ПЕРВЫМ (50 < 77) и забирал бы себе ВСЕ точки Москвы, включая центр
|
||||
# (55.75, 37.62) — она лежит в bbox_region обоих регионов одновременно. Старый
|
||||
# докстринг называл это «на случай, если когда-нибудь пересекутся» — случай
|
||||
# наступил прямо при добавлении региона 50, не гипотетически.
|
||||
# #3052: сам резолв региона идёт по настоящему полигону (see
|
||||
# _point_in_region_polygon), не по прямоугольнику — реальные админ-границы
|
||||
# 50 и 77 не пересекаются (Москва вырезана дырой из полигона области), так
|
||||
# что для КОНЕЧНОГО результата порядок обхода больше не обязателен: у точки
|
||||
# есть ровно один полигон-кандидат, bbox какого региона ни проверяй первым.
|
||||
# Порядок остаётся не как костыль корректности, а как ДЕШЁВЫЙ предварительный
|
||||
# отсев: bbox_region(50) (Московская область целиком, lat 54.20..56.96/
|
||||
# lon 35.14..40.21) геометрически СОДЕРЖИТ bbox_region(77) (Москва,
|
||||
# 55.10..56.10/36.80..38.10) как прямоугольники, а 50 < 77 по числовому коду.
|
||||
# Если проверять регионы в порядке `sorted(REGIONS)`, для точки в центре
|
||||
# Москвы bbox-отсев региона 50 пройдёт ПЕРВЫМ и завернёт в дорогой
|
||||
# point-in-polygon по 1 227 точкам области раньше, чем дело дойдёт до
|
||||
# компактного полигона Москвы (386 точек) — лишняя работа на каждый вызов,
|
||||
# не баг результата (полигон 50 всё равно отвергнет точку — она в дыре), но
|
||||
# systematic overhead на самом частом случае (Москва/область — соседи).
|
||||
#
|
||||
# Площадь bbox_region (см. `_bbox_area`) как ключ сортировки решает это БЕЗ
|
||||
# ручного списка: чем компактнее регион, тем раньше его проверяют, поэтому
|
||||
# вложенный регион (77 внутри 50) всегда выигрывает у объемлющего, а будущий
|
||||
# новый регион сам встанет в верную позицию по своей площади — правку этого
|
||||
# места повторять не придётся.
|
||||
# Площадь bbox_region (см. `_bbox_area`) как ключ сортировки решает и это без
|
||||
# ручного списка: чем компактнее регион, тем раньше его bbox-отсев и (при
|
||||
# совпадении) полигон проверяют, поэтому вложенный по bbox регион (77 внутри
|
||||
# 50) почти всегда получает свою точку дешевле, а новый регион сам встанет в
|
||||
# верную позицию по своей площади.
|
||||
_POINT_LOOKUP_ORDER: tuple[int, ...] = tuple(
|
||||
sorted(REGIONS, key=lambda code: (_bbox_area(REGIONS[code].bbox_region), code))
|
||||
)
|
||||
|
||||
|
||||
def region_for_point(lat: float, lon: float) -> Region | None:
|
||||
"""Регион покрытия, которому принадлежит точка (по bbox_region), или None.
|
||||
"""Регион покрытия, которому принадлежит точка, или None (вне охвата).
|
||||
|
||||
Обход — `_POINT_LOOKUP_ORDER` (компактный bbox_region раньше обширного), а
|
||||
не числовой код региона: код как ключ порядка ломается ровно на паре
|
||||
50/77, см. комментарий у `_POINT_LOOKUP_ORDER`.
|
||||
Два уровня отсева на каждого кандидата (порядок — `_POINT_LOOKUP_ORDER`,
|
||||
компактный bbox_region раньше обширного, см. комментарий там):
|
||||
1. `is_within_bbox` по `bbox_region` — дешёвый прямоугольный предфильтр,
|
||||
НЕ финальный ответ (прямоугольники Москвы и области пересекаются).
|
||||
2. Полигон (`_point_in_region_polygon`) — настоящая граница, решает
|
||||
результат. Если bbox прошёл, а полигон точку не принял (точка в
|
||||
прямоугольнике области, но не в её реальных границах — то есть,
|
||||
например, внутри вырезанной дыры Москвы), идём к следующему
|
||||
кандидату, а не возвращаем None сразу.
|
||||
Региона без загруженного полигона в реестре нет (все REGIONS покрыты
|
||||
boundaries.geojson.json) — на practике до fallback-ветки дело не доходит,
|
||||
но она есть, чтобы новый регион без границы не «пропадал» молча, а
|
||||
работал по старому bbox-поведению до того, как для него добавят полигон.
|
||||
"""
|
||||
for code in _POINT_LOOKUP_ORDER:
|
||||
if is_within_bbox(lat, lon, REGIONS[code].bbox_region):
|
||||
return REGIONS[code]
|
||||
region = REGIONS[code]
|
||||
if not is_within_bbox(lat, lon, region.bbox_region):
|
||||
continue
|
||||
if code not in _REGION_BOUNDARIES or _point_in_region_polygon(code, lat, lon):
|
||||
return region
|
||||
return None
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -50,7 +50,17 @@ def test_regions_do_not_overlap() -> None:
|
|||
— там же настоящий инвариант для вложенной пары: `region_for_point` обязан
|
||||
резолвить точку во ВЛАДЕЮЩИЙ, более специфичный регион по площади bbox, а не
|
||||
по числовому коду). 66 (Урал) физически за тысячи км от 50/77 — для него
|
||||
старый плоский инвариант «никто ни в кого не попадает» остаётся в силе."""
|
||||
старый плоский инвариант «никто ни в кого не попадает» остаётся в силе.
|
||||
|
||||
#3052: этот тест — про СЫРЫЕ прямоугольники (`is_within_bbox` /
|
||||
`bbox_region`), не про итоговый резолв точки. Переход `region_for_point`
|
||||
на настоящий полигон (см. regions._point_in_region_polygon) исключение
|
||||
здесь НЕ снимает: bbox_region(77) геометрически остаётся подмножеством
|
||||
bbox_region(50) как прямоугольник — полигон это не меняет, он работает
|
||||
ПОСЛЕ bbox-предфильтра, а не вместо него. Честный инвариант «точка резолвится
|
||||
ровно в один регион» теперь проверяется на уровне function-результата в
|
||||
test_3051_region_registry_moscow_oblast.py (Химки/Балашиха и девять
|
||||
контрольных точек issue #3052), не здесь."""
|
||||
nested_pairs = {(50, 77), (77, 50)}
|
||||
# Исключение из инварианта обязано опираться на ДОКАЗАННУЮ вложенность, а не
|
||||
# на допущение в докстринге: сначала проверяем, что bbox_region(50)
|
||||
|
|
|
|||
|
|
@ -13,6 +13,12 @@ bbox_region(77) (Москва) как прямоугольники, а 50 < 77
|
|||
Область на дату этого PR — тир обогащения пуст (frozenset()), в deals/listings
|
||||
0 строк (импорт Росреестра из FDW — отдельный PR). Тесты ниже проверяют РЕЕСТР
|
||||
(геометрию/классификацию), не данные.
|
||||
|
||||
#3052: резолв внутри `region_for_point` перестал быть чисто bbox-based —
|
||||
`_POINT_LOOKUP_ORDER` (регрессия ниже) остался прежним дешёвым предфильтром,
|
||||
но финальный ответ теперь даёт настоящий полигон границы региона. Тест на
|
||||
Химки/Балашиху обновлён под это (было «известное ограничение» bbox, теперь —
|
||||
исправленное поведение).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
|
@ -73,20 +79,20 @@ def test_point_outside_all_regions_is_none() -> None:
|
|||
assert res is None
|
||||
|
||||
|
||||
def test_known_limitation_khimki_balashikha_resolve_to_77_via_bbox() -> None:
|
||||
"""ИЗВЕСТНОЕ ОГРАНИЧЕНИЕ, не баг этого PR: bbox_region(77) — генеральный
|
||||
fallback-net Москвы, по долготе тянется до 38.10 (~13 км восточнее МКАД).
|
||||
Химки (55.91, 37.41) и Балашиха (55.7965, 37.9388) физически лежат внутри
|
||||
ЭТОГО прямоугольника, хотя административно это область. region_for_point
|
||||
(bbox-based) резолвит их в 77; region_by_city (точное имя) — правильно в
|
||||
50 (см. test_region_by_city_resolves_moscow_oblast_cities). Сузить
|
||||
bbox_region(77), чтобы это исправить, — отдельное решение вне скоупа
|
||||
ordering-фикса (риск задеть geocoder/estimator-потребителей 77 без
|
||||
возможности перепроверить их в этом PR)."""
|
||||
def test_khimki_balashikha_resolve_to_50_via_polygon() -> None:
|
||||
"""#3052: bbox_region(77) — генеральный fallback-net Москвы, по долготе
|
||||
тянется до 38.10 (~13 км восточнее МКАД). Химки (55.91, 37.41) и Балашиха
|
||||
(55.7965, 37.9388) физически лежат внутри ЭТОГО прямоугольника, хотя
|
||||
административно это область — до #3052 region_for_point (bbox-based)
|
||||
резолвил их в 77 («известное ограничение», см. issue #3052: 8 647
|
||||
московских лотов уходили в отказ по покрытию, а 11 172 областные сделки
|
||||
ложно попадали в московское ядро). С настоящим полигоном (bbox остался
|
||||
только дешёвым предфильтром — см. regions.region_for_point) обе точки
|
||||
физически вне контура Москвы → резолвятся в 50, как и region_by_city."""
|
||||
khimki = regions.region_for_point(55.91, 37.41)
|
||||
assert khimki is not None and khimki.code == 77
|
||||
assert khimki is not None and khimki.code == 50
|
||||
balashikha = regions.region_for_point(55.7965, 37.9388)
|
||||
assert balashikha is not None and balashikha.code == 77
|
||||
assert balashikha is not None and balashikha.code == 50
|
||||
|
||||
|
||||
# ── 2. region_by_city ─────────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -0,0 +1,68 @@
|
|||
"""#3052: `region_for_point` резолвит по настоящей границе региона (полигон
|
||||
OSM/Nominatim из `region_boundaries/boundaries.geojson.json`), не по
|
||||
прямоугольнику `bbox_region`.
|
||||
|
||||
До фикса прямоугольники Москвы (77) и области (50) пересекались — Химки,
|
||||
Реутов, Котельники, Люберцы резолвились в Москву (8 647 московских лотов
|
||||
получали отказ по покрытию, 11 172 областные сделки ложно попадали в
|
||||
московское ядро, при том что коридор ДКП фильтрует строго по коду региона).
|
||||
|
||||
Девять контрольных точек ниже — приёмка issue #3052, значения сверены
|
||||
`region_for_point` на реальных полигонах до мержа (ray casting с поддержкой
|
||||
дыр, см. regions._point_in_region_polygon)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
os.environ.setdefault("DATABASE_URL", "postgresql+psycopg://test:test@localhost:5432/test")
|
||||
|
||||
from app.services import regions
|
||||
|
||||
# (название, lat, lon, ожидаемый code региона или None)
|
||||
_CONTROL_POINTS: tuple[tuple[str, float, float, int | None], ...] = (
|
||||
("Химки", 55.8970, 37.4297, 50),
|
||||
("центр Москвы", 55.7558, 37.6173, 77),
|
||||
("Ломоносовский проспект", 55.7020, 37.5140, 77),
|
||||
("Серпухов", 54.9147, 37.4147, 50),
|
||||
("Балашиха", 55.7965, 37.9391, 50),
|
||||
("Зеленоград", 55.9840, 37.2140, 77),
|
||||
("Троицк (Новая Москва)", 55.4750, 37.3000, 77),
|
||||
("Екатеринбург", 56.8389, 60.6057, 66),
|
||||
("Пермь", 58.0105, 56.2502, None),
|
||||
)
|
||||
|
||||
|
||||
def test_nine_control_points_resolve_by_real_boundary() -> None:
|
||||
for name, lat, lon, expected_code in _CONTROL_POINTS:
|
||||
res = regions.region_for_point(lat, lon)
|
||||
got_code = res.code if res is not None else None
|
||||
assert got_code == expected_code, (
|
||||
f"{name} ({lat}, {lon}): ожидался регион {expected_code}, получен {got_code}"
|
||||
)
|
||||
|
||||
|
||||
def test_point_outside_all_boundaries_is_none() -> None:
|
||||
"""Пермь — вне всех трёх полигонов (не только вне bbox), инвариант отдельно
|
||||
от общего прогона выше — чтобы регрессия на None была явной сама по себе."""
|
||||
assert regions.region_for_point(58.0105, 56.2502) is None
|
||||
|
||||
|
||||
def test_ekaterinburg_unaffected_by_polygon_switch() -> None:
|
||||
"""Регион 66 физически за тысячи км от 50/77 — переход на полигон не должен
|
||||
был задеть его резолв вообще."""
|
||||
res = regions.region_for_point(56.8389, 60.6057)
|
||||
assert res is not None and res.code == 66
|
||||
|
||||
|
||||
def test_bbox_prefilter_never_rejects_what_polygon_would_accept() -> None:
|
||||
"""Инвариант «полигон вложен в свой bbox_region»: если бы это было не так,
|
||||
дешёвый bbox-предфильтр в region_for_point молча отбрасывал бы точки,
|
||||
которые честный point-in-polygon принял бы — тихая потеря покрытия."""
|
||||
for code, rings in regions._REGION_BOUNDARIES.items():
|
||||
bbox = regions.REGIONS[code].bbox_region
|
||||
for ring in rings:
|
||||
for lon, lat in ring:
|
||||
assert regions.is_within_bbox(lat, lon, bbox), (
|
||||
f"вершина полигона региона {code} ({lat}, {lon}) вне его bbox_region"
|
||||
)
|
||||
Loading…
Add table
Reference in a new issue