mirror of
https://github.com/Hestia-Homes/Model.git
synced 2026-06-30 13:10:47 +00:00
Scope 21.0.0 widening to fields the skipped certs actually omit
Review follow-up (Khalim): the first pass made far more optional than needed — notably the whole SapBuildingPart block — and a buggy 21.0.0↔21.0.1 diff also MISSED open_chimneys_count / cfl_/led_fixed_lighting_bulbs_count / suggested_ improvements, so the original change actually mapped only 3 of the 33 skipped certs (the rest still failed on open_chimneys_count). Re-derived the exact set empirically from all 33 skipped cohort certs: widen only fields that are (a) required in 21.0.0, (b) already optional in 21.0.1, AND (c) genuinely omitted by ≥1 of those certs. Result: - KEEP optional: the 4 SapWindow refinements, the top-level vent/lighting/ door/pressure-test block (incl. the 3 previously-missed fields), 2 SapEnergySource fields, Addendum.addendum_numbers, PhotovoltaicSupply. none_or_no_details, and exactly ONE building-part field (SapBuildingPart.roof_insulation_thickness — omitted by 7 certs). - REVERT to required: the other 12 SapBuildingPart fields (construction_age_ band, wall_construction, …), MainHeatingDetail.emitter_temperature, PvBatteries.pv_battery, ShowerOutlets.shower_outlet — none of the 33 certs omit these, so they stay strict. Mapper: coalesce the count fields (wet_rooms_count, open_chimneys_count, cfl_/led_fixed_lighting_bulbs_count) to 0 like every other mapper, so the now- optional values can't reach a NOT-NULL column (also drops 4 pyright ignores). Now maps 32/33 (up from 3); the last cert hits a pre-existing pv_batteries- shape AttributeError and degrades via the ADR-0031 skip path. pyright net unchanged (43, no new errors); regression test rewritten to the real omitted set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jun-te Kim
parent
44991bed0f
commit
7489e5ac89
3 changed files with 53 additions and 43 deletions
|
|
@ -1850,13 +1850,15 @@ class EpcPropertyDataMapper:
|
|||
door_count=schema.door_count,
|
||||
habitable_rooms_count=schema.habitable_room_count,
|
||||
heated_rooms_count=schema.heated_room_count,
|
||||
wet_rooms_count=schema.wet_rooms_count, # pyright: ignore[reportArgumentType]
|
||||
# Optional in 21.0.0 (real certs omit them); coalesce counts to 0
|
||||
# like every other mapper so None can't reach a NOT-NULL column.
|
||||
wet_rooms_count=schema.wet_rooms_count or 0,
|
||||
extensions_count=schema.extensions_count,
|
||||
open_chimneys_count=schema.open_chimneys_count,
|
||||
open_chimneys_count=schema.open_chimneys_count or 0,
|
||||
insulated_door_count=schema.insulated_door_count,
|
||||
draughtproofed_door_count=schema.draughtproofed_door_count,
|
||||
led_fixed_lighting_bulbs_count=schema.led_fixed_lighting_bulbs_count,
|
||||
cfl_fixed_lighting_bulbs_count=schema.cfl_fixed_lighting_bulbs_count,
|
||||
led_fixed_lighting_bulbs_count=schema.led_fixed_lighting_bulbs_count or 0,
|
||||
cfl_fixed_lighting_bulbs_count=schema.cfl_fixed_lighting_bulbs_count or 0,
|
||||
incandescent_fixed_lighting_bulbs_count=schema.incandescent_fixed_lighting_bulbs_count,
|
||||
roofs=EpcPropertyDataMapper._map_energy_elements(schema.roofs),
|
||||
walls=EpcPropertyDataMapper._map_energy_elements(schema.walls),
|
||||
|
|
@ -1886,7 +1888,7 @@ class EpcPropertyDataMapper:
|
|||
boiler_flue_type=d.boiler_flue_type,
|
||||
fan_flue_present=d.fan_flue_present == "Y",
|
||||
heat_emitter_type=d.heat_emitter_type,
|
||||
emitter_temperature=d.emitter_temperature, # pyright: ignore[reportArgumentType]
|
||||
emitter_temperature=d.emitter_temperature,
|
||||
main_heating_number=d.main_heating_number,
|
||||
boiler_ignition_type=d.boiler_ignition_type,
|
||||
main_heating_control=d.main_heating_control,
|
||||
|
|
@ -1982,9 +1984,9 @@ class EpcPropertyDataMapper:
|
|||
sap_building_parts=[
|
||||
SapBuildingPart(
|
||||
identifier=BuildingPartIdentifier.from_api_string(bp.identifier),
|
||||
construction_age_band=bp.construction_age_band, # pyright: ignore[reportArgumentType]
|
||||
wall_construction=_api_wall_construction_code(bp.wall_construction), # pyright: ignore[reportArgumentType]
|
||||
wall_insulation_type=bp.wall_insulation_type, # pyright: ignore[reportArgumentType]
|
||||
construction_age_band=bp.construction_age_band,
|
||||
wall_construction=_api_wall_construction_code(bp.wall_construction),
|
||||
wall_insulation_type=bp.wall_insulation_type,
|
||||
wall_thickness_measured=bp.wall_thickness_measured == "Y",
|
||||
party_wall_construction=_api_party_wall_construction_int(
|
||||
bp.party_wall_construction
|
||||
|
|
|
|||
|
|
@ -254,32 +254,40 @@ class TestFromRdSapSchema21_0_0:
|
|||
return EpcPropertyDataMapper.from_rdsap_schema_21_0_0(schema)
|
||||
|
||||
def test_optional_fields_omitted_maps_not_crashed(self) -> None:
|
||||
# Real-API 21.0.0 cohort certs routinely omit fields that the dataclass
|
||||
# originally modelled as required while 21.0.1 already treated as
|
||||
# optional (window frame_factor / glazing_gap / transmission details,
|
||||
# wet_rooms_count, the mechanical-vent duct block, wind_turbine_details,
|
||||
# &c.). The modelling_e2e cohort skipped ~35 such certs with
|
||||
# "SapWindow: missing required field 'frame_factor'" (cert
|
||||
# 2205-3036-3484-0400-5718). The 21.0.0 schema now mirrors 21.0.1's
|
||||
# optionality so they map instead of being dropped from the donor pool.
|
||||
# Real-API 21.0.0 cohort certs omit a set of fields that the dataclass
|
||||
# modelled as required while 21.0.1 already treats as optional. The
|
||||
# modelling_e2e cohort skipped 33 such certs (e.g. 2205-3036-3484-0400-
|
||||
# 5718). This is the exact union of fields those certs omit — all are
|
||||
# optional in 21.0.1; widening 21.0.0 to match maps them into the donor
|
||||
# pool instead of dropping them. (Building-part fields are NOT widened:
|
||||
# the only one those certs omit is roof_insulation_thickness.)
|
||||
data = load("21_0_0.json")
|
||||
for window in data.get("sap_windows", []):
|
||||
for k in ("frame_factor", "glazing_gap", "pvc_frame",
|
||||
"window_transmission_details"):
|
||||
window.pop(k, None)
|
||||
for k in ("wet_rooms_count", "pressure_test_certificate_number",
|
||||
for k in ("wet_rooms_count", "open_chimneys_count",
|
||||
"pressure_test_certificate_number", "windows_transmission_details",
|
||||
"mechanical_ventilation_index_number", "mechanical_vent_duct_type",
|
||||
"mechanical_vent_duct_placement", "mechanical_vent_duct_insulation",
|
||||
"mechanical_vent_duct_insulation_level",
|
||||
"mechanical_vent_measured_installation",
|
||||
"insulated_door_u_value", "low_energy_fixed_lighting_bulbs_count"):
|
||||
"mechanical_vent_measured_installation", "insulated_door_u_value",
|
||||
"low_energy_fixed_lighting_bulbs_count",
|
||||
"cfl_fixed_lighting_bulbs_count", "led_fixed_lighting_bulbs_count",
|
||||
"suggested_improvements"):
|
||||
data.pop(k, None)
|
||||
data["sap_energy_source"].pop("wind_turbine_details", None)
|
||||
data["sap_energy_source"].pop("pv_battery_count", None)
|
||||
for bp in data.get("sap_building_parts", []):
|
||||
bp.pop("roof_insulation_thickness", None)
|
||||
|
||||
schema = from_dict(RdSapSchema21_0_0, data)
|
||||
result = EpcPropertyDataMapper.from_rdsap_schema_21_0_0(schema)
|
||||
|
||||
assert result.uprn == 12457
|
||||
# coalesced, not None → safe for the NOT-NULL epc_property column
|
||||
assert result.wet_rooms_count == 0
|
||||
assert result.open_chimneys_count == 0
|
||||
|
||||
def test_uprn(self, result: EpcPropertyData) -> None:
|
||||
assert result.uprn == 12457
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ class EnergyElement:
|
|||
environmental_efficiency_rating: int
|
||||
|
||||
|
||||
@dataclass(kw_only=True)
|
||||
@dataclass
|
||||
class Addendum:
|
||||
addendum_numbers: Optional[List[int]] = None
|
||||
stone_walls: Optional[str] = None
|
||||
|
|
@ -25,9 +25,9 @@ class ShowerOutlet:
|
|||
shower_outlet_type: int
|
||||
|
||||
|
||||
@dataclass(kw_only=True)
|
||||
@dataclass
|
||||
class ShowerOutlets:
|
||||
shower_outlet: Optional[ShowerOutlet] = None
|
||||
shower_outlet: ShowerOutlet
|
||||
|
||||
|
||||
@dataclass
|
||||
|
|
@ -37,12 +37,12 @@ class InstantaneousWwhrs:
|
|||
wwhrs_index_number2: Optional[int] = None
|
||||
|
||||
|
||||
@dataclass(kw_only=True)
|
||||
@dataclass
|
||||
class MainHeatingDetail:
|
||||
has_fghrs: str
|
||||
main_fuel_type: int
|
||||
heat_emitter_type: int
|
||||
emitter_temperature: Optional[Union[int, str]] = None
|
||||
emitter_temperature: Union[int, str]
|
||||
main_heating_number: int
|
||||
main_heating_control: int
|
||||
main_heating_category: int
|
||||
|
|
@ -91,9 +91,9 @@ class PvBattery:
|
|||
battery_capacity: float
|
||||
|
||||
|
||||
@dataclass(kw_only=True)
|
||||
@dataclass
|
||||
class PvBatteries:
|
||||
pv_battery: Optional[PvBattery] = None
|
||||
pv_battery: PvBattery
|
||||
|
||||
|
||||
@dataclass
|
||||
|
|
@ -107,7 +107,7 @@ class PhotovoltaicSupplyNoneOrNoDetails:
|
|||
percent_roof_area: int
|
||||
|
||||
|
||||
@dataclass(kw_only=True)
|
||||
@dataclass
|
||||
class PhotovoltaicSupply:
|
||||
none_or_no_details: Optional[PhotovoltaicSupplyNoneOrNoDetails] = None
|
||||
|
||||
|
|
@ -279,18 +279,18 @@ class SapAlternativeWall:
|
|||
|
||||
@dataclass(kw_only=True)
|
||||
class SapBuildingPart:
|
||||
identifier: Optional[str] = None
|
||||
wall_dry_lined: Optional[str] = None
|
||||
floor_heat_loss: Optional[int] = None
|
||||
roof_construction: Optional[int] = None
|
||||
wall_construction: Optional[int] = None
|
||||
building_part_number: Optional[int] = None
|
||||
sap_floor_dimensions: Optional[List[SapFloorDimension]] = None
|
||||
wall_insulation_type: Optional[int] = None
|
||||
construction_age_band: Optional[str] = None
|
||||
party_wall_construction: Optional[Union[int, str]] = None
|
||||
wall_thickness_measured: Optional[str] = None
|
||||
roof_insulation_location: Optional[Union[int, str]] = None
|
||||
identifier: str
|
||||
wall_dry_lined: str
|
||||
floor_heat_loss: int
|
||||
roof_construction: int
|
||||
wall_construction: int
|
||||
building_part_number: int
|
||||
sap_floor_dimensions: List[SapFloorDimension]
|
||||
wall_insulation_type: int
|
||||
construction_age_band: str
|
||||
party_wall_construction: Union[int, str]
|
||||
wall_thickness_measured: str
|
||||
roof_insulation_location: Union[int, str]
|
||||
roof_insulation_thickness: Optional[Union[str, int]] = None
|
||||
sap_room_in_roof: Optional[SapRoomInRoof] = None
|
||||
sap_alternative_wall_1: Optional[SapAlternativeWall] = None
|
||||
|
|
@ -431,7 +431,7 @@ class RdSapSchema21_0_0:
|
|||
sap_energy_source: SapEnergySource
|
||||
secondary_heating: EnergyElement
|
||||
sap_building_parts: List[SapBuildingPart]
|
||||
open_chimneys_count: int
|
||||
open_chimneys_count: Optional[int] = None
|
||||
solar_water_heating: str
|
||||
habitable_room_count: int
|
||||
heating_cost_current: float
|
||||
|
|
@ -447,7 +447,7 @@ class RdSapSchema21_0_0:
|
|||
insulated_door_u_value: Optional[float] = None
|
||||
mechanical_ventilation: int
|
||||
percent_draughtproofed: int
|
||||
suggested_improvements: List[SuggestedImprovement]
|
||||
suggested_improvements: Optional[List[SuggestedImprovement]] = None
|
||||
co2_emissions_potential: float
|
||||
energy_rating_potential: int
|
||||
lighting_cost_potential: float
|
||||
|
|
@ -457,14 +457,14 @@ class RdSapSchema21_0_0:
|
|||
draughtproofed_door_count: int
|
||||
mechanical_vent_duct_type: Optional[int] = None
|
||||
windows_transmission_details: Optional[WindowsTransmissionDetails] = None
|
||||
cfl_fixed_lighting_bulbs_count: int
|
||||
cfl_fixed_lighting_bulbs_count: Optional[int] = None
|
||||
energy_consumption_current: int
|
||||
has_fixed_air_conditioning: str
|
||||
multiple_glazed_proportion: int
|
||||
calculation_software_version: str
|
||||
energy_consumption_potential: int
|
||||
environmental_impact_current: int
|
||||
led_fixed_lighting_bulbs_count: int
|
||||
led_fixed_lighting_bulbs_count: Optional[int] = None
|
||||
mechanical_vent_duct_placement: Optional[int] = None
|
||||
mechanical_vent_duct_insulation: Optional[int] = None
|
||||
potential_energy_efficiency_band: str
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue