diff --git a/backend/address2UPRN/main.py b/backend/address2UPRN/main.py index 136f4344b..6b66dfdbc 100644 --- a/backend/address2UPRN/main.py +++ b/backend/address2UPRN/main.py @@ -16,11 +16,11 @@ from datetime import datetime from backend.utils.addressMatch import AddressMatch from backend.address2UPRN.scoring import all_uprns_match, rank_address_similarity -from datatypes.epc.domain.historic_epc_matching import ( - match_addresses_for_postcode, -) from infrastructure.epc_client.epc_client_service import EpcClientService -from datatypes.epc.domain.historic_epc_matching import ScoredHistoricEpc +from repositories.historic_epc.historic_epc_resolver import HistoricEpcResolver +from repositories.historic_epc.historic_epc_s3_repository import ( + HistoricEpcS3Repository, +) logger = setup_logger() @@ -45,26 +45,14 @@ def get_uprn_from_historic_epc( """Resolve a UPRN via historic EPC S3 data. Returns (uprn, address, lexiscore) when the historic dataset agrees on a - single rank-1 UPRN, None otherwise (missing postcode file, zero score, - or ambiguous top rank). The score gate is `unambiguous_uprn`'s own - (score > 0); the 0.7 heuristic used for the new-EPC source isn't applied - here because historic addresses use a more verbose format that - systematically depresses lexiscores. + single rank-1 UPRN, None otherwise (no stored data, zero score, or + ambiguous top rank). The score gate is `unambiguous_uprn`'s own (score > 0); + the 0.7 heuristic used for the new-EPC source isn't applied here because + historic addresses use a more verbose format that systematically depresses + lexiscores. """ - - try: - result = match_addresses_for_postcode(user_inputed_address, postcode) - except FileNotFoundError: - return None - - uprn: Optional[str] = result.unambiguous_uprn() - if not uprn or uprn == "nan": - return None - - top: Optional[ScoredHistoricEpc] = result.top() - if top is None: - return None - return uprn, top.record.address, top.lexiscore + repo = HistoricEpcS3Repository() + return HistoricEpcResolver(repo).resolve_uprn(user_inputed_address, postcode) def get_uprn_with_epc_df( diff --git a/datatypes/epc/domain/historic_epc_matching.py b/datatypes/epc/domain/historic_epc_matching.py index 09da3bf9a..e64ed4143 100644 --- a/datatypes/epc/domain/historic_epc_matching.py +++ b/datatypes/epc/domain/historic_epc_matching.py @@ -2,13 +2,11 @@ from dataclasses import dataclass from typing import Optional import pandas as pd -from botocore.exceptions import ClientError from backend.address2UPRN.scoring import rank_address_similarity from backend.utils.addressMatch import AddressMatch from datatypes.epc.domain.historic_epc import HistoricEpc from utils.pandas_utils import pandas_cell_to_str -from utils.s3 import parse_s3_uri, read_csv_gz_from_s3 DEFAULT_S3_ROOT = "s3://retrofit-data-dev/historical_epc" @@ -73,6 +71,7 @@ def rank_historic_epc( df = pd.DataFrame( { + "_pos": range(len(records)), address_column: [r.address for r in records], uprn_column: [r.uprn for r in records], } @@ -85,11 +84,11 @@ def rank_historic_epc( ) return [ ScoredHistoricEpc( - record=records[i], + record=records[int(row["_pos"])], lexiscore=float(row["lexiscore"]), lexirank=int(row["lexirank"]), ) - for i, row in scored.iterrows() + for _, row in scored.iterrows() ] @@ -107,39 +106,19 @@ def match_addresses_for_postcode( postcode: str, *, s3_root: str = DEFAULT_S3_ROOT, - address_column: str = "ADDRESS", - uprn_column: str = "UPRN", ) -> HistoricEpcMatches: - if not user_address: - raise ValueError("user_address must be non-empty") + """Score a postcode's historic EPCs against ``user_address``. - pc = _sanitise_postcode(postcode) - bucket, root_prefix = parse_s3_uri(s3_root) - key = f"{root_prefix.rstrip('/')}/{pc}/data.csv.gz" - - try: - df = read_csv_gz_from_s3(bucket, key) - except ClientError as e: - if e.response.get("Error", {}).get("Code") in ("NoSuchKey", "404"): - raise FileNotFoundError( - f"No historic EPC data at s3://{bucket}/{key}" - ) from e - raise - - scored = rank_address_similarity( - df, - user_address=user_address, - address_column=address_column, - uprn_column=uprn_column, + A thin composition seam over the historic-EPC repository + resolver; the S3 + read and scoring live there. A postcode with no stored shard yields empty + matches (not FileNotFoundError); an unusable postcode raises PostcodeNotFound. + Imported lazily so the domain layer doesn't import the repositories layer at + module load (the repository depends on this module). + """ + from repositories.historic_epc.historic_epc_resolver import HistoricEpcResolver + from repositories.historic_epc.historic_epc_s3_repository import ( + HistoricEpcS3Repository, ) - matches = [ - ScoredHistoricEpc( - record=_map_historic_epc_pandas_row_to_domain(row), - lexiscore=float(row["lexiscore"]), - lexirank=int(row["lexirank"]), - ) - for _, row in scored.iterrows() - ] - - return HistoricEpcMatches(user_address=user_address, postcode=pc, matches=matches) + repo = HistoricEpcS3Repository(s3_root=s3_root) + return HistoricEpcResolver(repo).match(user_address, postcode) diff --git a/datatypes/epc/domain/tests/test_historic_epc_matching.py b/datatypes/epc/domain/tests/test_historic_epc_matching.py index ce86e5c09..d335fbc3d 100644 --- a/datatypes/epc/domain/tests/test_historic_epc_matching.py +++ b/datatypes/epc/domain/tests/test_historic_epc_matching.py @@ -13,6 +13,7 @@ from datatypes.epc.domain.historic_epc_matching import ( _sanitise_postcode, match_addresses_for_postcode, ) +from repositories.historic_epc import historic_epc_s3_repository as repo_mod # Columns required by the HistoricEpc dataclass (lower-cased CSV columns). # The matcher only reads ADDRESS + UPRN to score; everything else is filled @@ -135,7 +136,9 @@ def patch_postcode_valid(): @pytest.fixture def patch_read(): - with patch.object(matcher_mod, "read_csv_gz_from_s3") as m: + # match_addresses_for_postcode now reads via the historic-EPC repository, so + # the S3 read is patched where the repository binds it. + with patch.object(repo_mod, "read_csv_gz_from_s3") as m: yield m @@ -216,14 +219,17 @@ class TestMatchAddressesForPostcode: "my-bucket", "some/prefix/AB338AL/data.csv.gz" ) - def test_no_such_key_translates_to_filenotfound( + def test_missing_postcode_object_yields_empty_matches( self, patch_read, patch_postcode_valid ): + # A valid postcode with no stored shard is a normal miss, not an error: + # empty matches, not FileNotFoundError. patch_read.side_effect = ClientError( {"Error": {"Code": "NoSuchKey", "Message": "missing"}}, "GetObject" ) - with pytest.raises(FileNotFoundError): - match_addresses_for_postcode("47 Gordon Road", "AB33 8AL") + result = match_addresses_for_postcode("47 Gordon Road", "AB33 8AL") + assert result.matches == [] + assert result.unambiguous_uprn() is None def test_other_client_error_propagates(self, patch_read, patch_postcode_valid): patch_read.side_effect = ClientError( diff --git a/repositories/historic_epc/historic_epc_s3_repository.py b/repositories/historic_epc/historic_epc_s3_repository.py index ce735802e..520a9dec7 100644 --- a/repositories/historic_epc/historic_epc_s3_repository.py +++ b/repositories/historic_epc/historic_epc_s3_repository.py @@ -1,6 +1,7 @@ from __future__ import annotations from collections.abc import Callable +from typing import Optional import pandas as pd from botocore.exceptions import ClientError @@ -29,10 +30,12 @@ class HistoricEpcS3Repository(HistoricEpcRepository): def __init__( self, - read_csv_gz: CsvGzReader = read_csv_gz_from_s3, + read_csv_gz: Optional[CsvGzReader] = None, s3_root: str = DEFAULT_S3_ROOT, ) -> None: - self._read_csv_gz = read_csv_gz + # Resolve the default reader at call time (not import time) so it stays + # patchable and the captured default can't go stale. + self._read_csv_gz: CsvGzReader = read_csv_gz or read_csv_gz_from_s3 self._s3_root = s3_root def get_for_postcode(self, postcode: str) -> list[HistoricEpc]: