From 246834fac0fd2ae250582fb78aeca0093c30efd2 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Sat, 4 Jul 2026 14:58:09 +0000 Subject: [PATCH] =?UTF-8?q?Map=20shard=20rows=20to=20HistoricEpc=20field-b?= =?UTF-8?q?y-field=20so=20every=20column=20is=20pyright-checked=20?= =?UTF-8?q?=F0=9F=9F=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The row→domain mapper now names all 93 constructor arguments explicitly instead of splatting a lowercased dict, takes a plain Mapping (a DataFrame.to_dict("records") row) instead of a pandas Series, and ignores columns the domain type doesn't know. A missing/renamed CSV column fails loudly as a KeyError at the row. Both iterrows() call sites move to to_dict("records") — pandas-stubs types iterrows' Series unparameterized, which strict mode rejects. pandas-stubs + boto3-stubs[s3] make the stack check clean: pyright strict is now 0 errors across the PR's files. Co-Authored-By: Claude Opus 4.8 --- .devcontainer/backend/requirements.txt | 2 +- datatypes/epc/domain/historic_epc_matching.py | 135 ++++++++++++++++-- infrastructure/s3/gzip_csv_s3_client.py | 6 +- .../historic_epc_s3_repository.py | 16 ++- 4 files changed, 139 insertions(+), 20 deletions(-) diff --git a/.devcontainer/backend/requirements.txt b/.devcontainer/backend/requirements.txt index f98b167e0..784f62ca3 100644 --- a/.devcontainer/backend/requirements.txt +++ b/.devcontainer/backend/requirements.txt @@ -27,7 +27,7 @@ pytest-postgresql moto[s3,sqs]==5.0.28 # mock_aws (moto 5.x) for S3/SQS in orchestration tests # Formatting black==26.1.0 -boto3-stubs +boto3-stubs[s3] # typed boto3.client("s3") for the S3 repositories openai # Type checking — strict pyright gate (CLAUDE.md). The pip `pyright` wrapper uses # the container's Node. pandas-stubs lets pandas-typed modules check cleanly diff --git a/datatypes/epc/domain/historic_epc_matching.py b/datatypes/epc/domain/historic_epc_matching.py index 20fa04997..73a685fc6 100644 --- a/datatypes/epc/domain/historic_epc_matching.py +++ b/datatypes/epc/domain/historic_epc_matching.py @@ -1,5 +1,6 @@ +from collections.abc import Hashable, Mapping from dataclasses import dataclass -from typing import Optional +from typing import Any, Optional import pandas as pd @@ -9,20 +10,118 @@ from utils.pandas_utils import pandas_cell_to_str DEFAULT_S3_ROOT = "s3://retrofit-data-dev/historical_epc" -_EXTRA_COLS = {"lexiscore", "lexirank"} +def map_historic_epc_row_to_domain(row: Mapping[Hashable, Any]) -> HistoricEpc: + """Map one historic-EPC shard row (upper-cased CSV columns) to the domain. + + Field-by-field so pyright checks every constructor argument: a missing or + renamed CSV column fails loudly here (KeyError) rather than surfacing as a + half-built record, and columns the domain type doesn't know are ignored. + Takes a plain mapping (a ``DataFrame.to_dict("records")`` row), not a + pandas Series, so the signature carries no pandas types. + """ + + def cell(column: str) -> str: + return pandas_cell_to_str(row[column]) -def map_historic_epc_pandas_row_to_domain(row: pd.Series) -> HistoricEpc: - kwargs = { - col.lower(): pandas_cell_to_str(val) - for col, val in row.items() - if col.lower() not in _EXTRA_COLS - } # pandas reads an all-integer UPRN column as float, so the cell stringifies # to "151020766.0"; the domain UPRN is the bare integer string. - uprn = kwargs.get("uprn", "") - kwargs["uprn"] = uprn[:-2] if uprn.endswith(".0") else uprn - return HistoricEpc(**kwargs) + uprn = cell("UPRN") + return HistoricEpc( + lmk_key=cell("LMK_KEY"), + address1=cell("ADDRESS1"), + address2=cell("ADDRESS2"), + address3=cell("ADDRESS3"), + postcode=cell("POSTCODE"), + building_reference_number=cell("BUILDING_REFERENCE_NUMBER"), + current_energy_rating=cell("CURRENT_ENERGY_RATING"), + potential_energy_rating=cell("POTENTIAL_ENERGY_RATING"), + current_energy_efficiency=cell("CURRENT_ENERGY_EFFICIENCY"), + potential_energy_efficiency=cell("POTENTIAL_ENERGY_EFFICIENCY"), + property_type=cell("PROPERTY_TYPE"), + built_form=cell("BUILT_FORM"), + inspection_date=cell("INSPECTION_DATE"), + local_authority=cell("LOCAL_AUTHORITY"), + constituency=cell("CONSTITUENCY"), + county=cell("COUNTY"), + lodgement_date=cell("LODGEMENT_DATE"), + transaction_type=cell("TRANSACTION_TYPE"), + environment_impact_current=cell("ENVIRONMENT_IMPACT_CURRENT"), + environment_impact_potential=cell("ENVIRONMENT_IMPACT_POTENTIAL"), + energy_consumption_current=cell("ENERGY_CONSUMPTION_CURRENT"), + energy_consumption_potential=cell("ENERGY_CONSUMPTION_POTENTIAL"), + co2_emissions_current=cell("CO2_EMISSIONS_CURRENT"), + co2_emiss_curr_per_floor_area=cell("CO2_EMISS_CURR_PER_FLOOR_AREA"), + co2_emissions_potential=cell("CO2_EMISSIONS_POTENTIAL"), + lighting_cost_current=cell("LIGHTING_COST_CURRENT"), + lighting_cost_potential=cell("LIGHTING_COST_POTENTIAL"), + heating_cost_current=cell("HEATING_COST_CURRENT"), + heating_cost_potential=cell("HEATING_COST_POTENTIAL"), + hot_water_cost_current=cell("HOT_WATER_COST_CURRENT"), + hot_water_cost_potential=cell("HOT_WATER_COST_POTENTIAL"), + total_floor_area=cell("TOTAL_FLOOR_AREA"), + energy_tariff=cell("ENERGY_TARIFF"), + mains_gas_flag=cell("MAINS_GAS_FLAG"), + floor_level=cell("FLOOR_LEVEL"), + flat_top_storey=cell("FLAT_TOP_STOREY"), + flat_storey_count=cell("FLAT_STOREY_COUNT"), + main_heating_controls=cell("MAIN_HEATING_CONTROLS"), + multi_glaze_proportion=cell("MULTI_GLAZE_PROPORTION"), + glazed_type=cell("GLAZED_TYPE"), + glazed_area=cell("GLAZED_AREA"), + extension_count=cell("EXTENSION_COUNT"), + number_habitable_rooms=cell("NUMBER_HABITABLE_ROOMS"), + number_heated_rooms=cell("NUMBER_HEATED_ROOMS"), + low_energy_lighting=cell("LOW_ENERGY_LIGHTING"), + number_open_fireplaces=cell("NUMBER_OPEN_FIREPLACES"), + hotwater_description=cell("HOTWATER_DESCRIPTION"), + hot_water_energy_eff=cell("HOT_WATER_ENERGY_EFF"), + hot_water_env_eff=cell("HOT_WATER_ENV_EFF"), + floor_description=cell("FLOOR_DESCRIPTION"), + floor_energy_eff=cell("FLOOR_ENERGY_EFF"), + floor_env_eff=cell("FLOOR_ENV_EFF"), + windows_description=cell("WINDOWS_DESCRIPTION"), + windows_energy_eff=cell("WINDOWS_ENERGY_EFF"), + windows_env_eff=cell("WINDOWS_ENV_EFF"), + walls_description=cell("WALLS_DESCRIPTION"), + walls_energy_eff=cell("WALLS_ENERGY_EFF"), + walls_env_eff=cell("WALLS_ENV_EFF"), + secondheat_description=cell("SECONDHEAT_DESCRIPTION"), + sheating_energy_eff=cell("SHEATING_ENERGY_EFF"), + sheating_env_eff=cell("SHEATING_ENV_EFF"), + roof_description=cell("ROOF_DESCRIPTION"), + roof_energy_eff=cell("ROOF_ENERGY_EFF"), + roof_env_eff=cell("ROOF_ENV_EFF"), + mainheat_description=cell("MAINHEAT_DESCRIPTION"), + mainheat_energy_eff=cell("MAINHEAT_ENERGY_EFF"), + mainheat_env_eff=cell("MAINHEAT_ENV_EFF"), + mainheatcont_description=cell("MAINHEATCONT_DESCRIPTION"), + mainheatc_energy_eff=cell("MAINHEATC_ENERGY_EFF"), + mainheatc_env_eff=cell("MAINHEATC_ENV_EFF"), + lighting_description=cell("LIGHTING_DESCRIPTION"), + lighting_energy_eff=cell("LIGHTING_ENERGY_EFF"), + lighting_env_eff=cell("LIGHTING_ENV_EFF"), + main_fuel=cell("MAIN_FUEL"), + wind_turbine_count=cell("WIND_TURBINE_COUNT"), + heat_loss_corridor=cell("HEAT_LOSS_CORRIDOR"), + unheated_corridor_length=cell("UNHEATED_CORRIDOR_LENGTH"), + floor_height=cell("FLOOR_HEIGHT"), + photo_supply=cell("PHOTO_SUPPLY"), + solar_water_heating_flag=cell("SOLAR_WATER_HEATING_FLAG"), + mechanical_ventilation=cell("MECHANICAL_VENTILATION"), + address=cell("ADDRESS"), + local_authority_label=cell("LOCAL_AUTHORITY_LABEL"), + constituency_label=cell("CONSTITUENCY_LABEL"), + posttown=cell("POSTTOWN"), + construction_age_band=cell("CONSTRUCTION_AGE_BAND"), + lodgement_datetime=cell("LODGEMENT_DATETIME"), + tenure=cell("TENURE"), + fixed_lighting_outlets_count=cell("FIXED_LIGHTING_OUTLETS_COUNT"), + low_energy_fixed_light_count=cell("LOW_ENERGY_FIXED_LIGHT_COUNT"), + uprn=uprn[:-2] if uprn.endswith(".0") else uprn, + uprn_source=cell("UPRN_SOURCE"), + report_type=cell("REPORT_TYPE"), + ) @dataclass(frozen=True) @@ -81,13 +180,19 @@ def rank_historic_epc( address_column=address_column, uprn_column=uprn_column, ) + # pandas-stubs' to_dict overloads carry bare generics of their own, so + # strict mode flags the member access; the rows annotation keeps the + # comprehension below fully typed. + scored_rows: list[dict[Hashable, Any]] = scored.to_dict( # pyright: ignore[reportUnknownMemberType] + orient="records" + ) return [ ScoredHistoricEpc( - record=records[int(row["_pos"])], - lexiscore=float(row["lexiscore"]), - lexirank=int(row["lexirank"]), + record=records[int(scored_row["_pos"])], + lexiscore=float(scored_row["lexiscore"]), + lexirank=int(scored_row["lexirank"]), ) - for _, row in scored.iterrows() + for scored_row in scored_rows ] diff --git a/infrastructure/s3/gzip_csv_s3_client.py b/infrastructure/s3/gzip_csv_s3_client.py index acc3fd940..ee1172556 100644 --- a/infrastructure/s3/gzip_csv_s3_client.py +++ b/infrastructure/s3/gzip_csv_s3_client.py @@ -19,4 +19,8 @@ class GzipCsvS3Client(S3Client): def read_csv_gz(self, key: str) -> pd.DataFrame: raw = self.get_object(key) - return pd.read_csv(BytesIO(raw), compression="gzip", low_memory=False) + # pandas-stubs' read_csv overloads carry bare generics of their own, so + # strict mode flags the member access; the call and return are typed. + return pd.read_csv( # pyright: ignore[reportUnknownMemberType] + BytesIO(raw), compression="gzip", low_memory=False + ) diff --git a/repositories/historic_epc/historic_epc_s3_repository.py b/repositories/historic_epc/historic_epc_s3_repository.py index fd4c857b1..c8409ff80 100644 --- a/repositories/historic_epc/historic_epc_s3_repository.py +++ b/repositories/historic_epc/historic_epc_s3_repository.py @@ -1,12 +1,13 @@ from __future__ import annotations +from collections.abc import Hashable from typing import Any from botocore.exceptions import ClientError from datatypes.epc.domain.historic_epc import HistoricEpc from datatypes.epc.domain.historic_epc_matching import ( - map_historic_epc_pandas_row_to_domain, + map_historic_epc_row_to_domain, ) from domain.postcode import Postcode from infrastructure.s3.gzip_csv_s3_client import GzipCsvS3Client @@ -47,7 +48,10 @@ class HistoricEpcS3Repository(HistoricEpcRepository): import boto3 bucket, root_prefix = parse_s3_uri(s3_root) - boto_s3: Any = boto3.client("s3") # pyright: ignore[reportUnknownMemberType] + # boto3-stubs types client("s3") via an overload set in which services + # without installed stubs return Unknown, so strict mode flags the + # member access; the "s3" overload itself resolves to a typed S3Client. + boto_s3 = boto3.client("s3") # pyright: ignore[reportUnknownMemberType] return cls(GzipCsvS3Client(boto_s3, bucket), root_prefix) def get_for_postcode(self, postcode: Postcode) -> list[HistoricEpc]: @@ -60,4 +64,10 @@ class HistoricEpcS3Repository(HistoricEpcRepository): if e.response.get("Error", {}).get("Code") in ("NoSuchKey", "404"): return [] raise - return [map_historic_epc_pandas_row_to_domain(row) for _, row in df.iterrows()] + # pandas-stubs' to_dict overloads carry bare generics of their own, so + # strict mode flags the member access; the rows annotation keeps the + # downstream mapping fully typed. + rows: list[dict[Hashable, Any]] = df.to_dict( # pyright: ignore[reportUnknownMemberType] + orient="records" + ) + return [map_historic_epc_row_to_domain(row) for row in rows]