From fd9752b981e94d079718c220896d16ceed601cd0 Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Tue, 10 Feb 2026 14:46:36 +0000 Subject: [PATCH 01/13] increase openpyxl version in sal devcontainer, and fix import path --- .devcontainer/asset_list/requirements.txt | 2 +- asset_list/app.py | 53 ++++------------------- 2 files changed, 9 insertions(+), 46 deletions(-) diff --git a/.devcontainer/asset_list/requirements.txt b/.devcontainer/asset_list/requirements.txt index 0640f2c9..33cc4c2e 100644 --- a/.devcontainer/asset_list/requirements.txt +++ b/.devcontainer/asset_list/requirements.txt @@ -7,7 +7,7 @@ mangum==0.19.0 # AWS boto3==1.35.44 # Data -openpyxl==3.1.2 +openpyxl==3.1.5 # Basic pytz uvicorn[standard] diff --git a/asset_list/app.py b/asset_list/app.py index b46254f9..a18feeaf 100644 --- a/asset_list/app.py +++ b/asset_list/app.py @@ -13,7 +13,7 @@ from asset_list.utils import get_data from dotenv import load_dotenv from backend.SearchEpc import SearchEpc -load_dotenv(dotenv_path="backend/.env") +load_dotenv(dotenv_path="../backend/.env") EPC_AUTH_TOKEN = os.getenv( "EPC_AUTH_TOKEN", ) @@ -69,51 +69,14 @@ def app(): Property UPRN """ - data_folder = "/Users/khalimconn-kowlessar/Documents/hestia/Customers/Hackney" - data_filename = "Domna SHF Wave 3 (3).xlsx" - sheet_name = "Domna Wave 3" - postcode_column = "Postcode" - address1_column = "Address 1" - address1_method = None - fulladdress_column = None - address_cols_to_concat = ["Address 1"] - missing_postcodes_method = None - landlord_year_built = "Construction Years" - landlord_os_uprn = "UPRN" - landlord_property_type = "Type" - landlord_built_form = "Attachment" - landlord_wall_construction = "Wall type" - landlord_roof_construction = None - landlord_heating_system = None - landlord_existing_pv = None - landlord_property_id = "Row ID" - landlord_sap = None - outcomes_filename = None - outcomes_sheetname = None - outcomes_postcode = None - outcomes_houseno = None - outcomes_id = None - outcomes_address = None - master_filepaths = [] - master_id_colnames = [] - master_to_asset_list_filepath = None - phase = False - ecosurv_landlords = None - asset_list_header = 0 - landlord_block_reference = None - - # Peabody data for cleaning - data_folder = ( - "/Users/khalimconn-kowlessar/Documents/hestia/Customers/Peabody/Nov 2025 Consulting " - "Project/data_validation" - ) - data_filename = "to_standardise_uprns.xlsx" + data_folder = "/workspaces/home/Downloads" + data_filename = "Anchor 1.xlsx" sheet_name = "Sheet1" postcode_column = "Postcode" - address1_column = None - address1_method = "house_number_extraction" - fulladdress_column = "Address" - address_cols_to_concat = None + address1_column = "House Number" + address1_method = None + fulladdress_column = None + address_cols_to_concat = ["House Number", "Address Line 1", "Address Line 2"] missing_postcodes_method = None landlord_year_built = None landlord_os_uprn = None @@ -123,7 +86,7 @@ def app(): landlord_roof_construction = None landlord_heating_system = None landlord_existing_pv = None - landlord_property_id = "LLUPRN" + landlord_property_id = "Landlord Property ID" landlord_sap = None outcomes_filename = None outcomes_sheetname = None From 0028fe72990c344f81d673ef457b937be9854ace Mon Sep 17 00:00:00 2001 From: Daniel Roth Date: Tue, 10 Feb 2026 14:52:27 +0000 Subject: [PATCH 02/13] revert changes to asset_list/app.py --- asset_list/app.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/asset_list/app.py b/asset_list/app.py index 01062261..30172121 100644 --- a/asset_list/app.py +++ b/asset_list/app.py @@ -13,7 +13,7 @@ from asset_list.utils import get_data from dotenv import load_dotenv from backend.SearchEpc import SearchEpc -load_dotenv(dotenv_path="../backend/.env") +load_dotenv(dotenv_path="backend/.env") EPC_AUTH_TOKEN = os.getenv( "EPC_AUTH_TOKEN", ) @@ -123,7 +123,7 @@ def app(): landlord_roof_construction = None landlord_heating_system = None landlord_existing_pv = None - landlord_property_id = "Landlord Property ID" + landlord_property_id = "LLUPRN" landlord_sap = None outcomes_filename = None outcomes_sheetname = None @@ -275,7 +275,7 @@ def app(): if skip is not None and not force_retrieve_data: if i <= skip: continue - chunk = asset_list.standardised_asset_list[i : i + chunk_size] + chunk = asset_list.standardised_asset_list[i: i + chunk_size] epc_data_chunk, errors_chunk, no_epc_chunk = get_data( df=chunk, row_id_name=asset_list.DOMNA_PROPERTY_ID, @@ -418,7 +418,7 @@ def app(): # Retrieve just the data we need epc_df = epc_df[ [asset_list.DOMNA_PROPERTY_ID] + list(asset_list.EPC_API_DATA_NAMES.keys()) - ].rename(columns=asset_list.EPC_API_DATA_NAMES) + ].rename(columns=asset_list.EPC_API_DATA_NAMES) # Look for columns not in the find my EPC data, which will have happened if we didn't # retrieve it in the first place @@ -435,7 +435,7 @@ def app(): find_my_epc_data[ [asset_list.DOMNA_PROPERTY_ID, "epc_has_floor_recommendation"] + list(asset_list.FIND_EPC_DATA_NAMES.keys()) - ].rename(columns=asset_list.FIND_EPC_DATA_NAMES), + ].rename(columns=asset_list.FIND_EPC_DATA_NAMES), how="left", on=asset_list.DOMNA_PROPERTY_ID, ) From f6f5f5cd9b922fa6b7b4903a846e6598088d0fdc Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Tue, 10 Feb 2026 16:45:51 +0000 Subject: [PATCH 03/13] safetly do not deploy --- .github/workflows/_build_image.yml | 2 +- .github/workflows/_deploy_lambda.yml | 18 +++++++++++++++ .github/workflows/deploy_terraform.yml | 32 ++++++++++++++++++++------ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/.github/workflows/_build_image.yml b/.github/workflows/_build_image.yml index 408c0319..641e31f9 100644 --- a/.github/workflows/_build_image.yml +++ b/.github/workflows/_build_image.yml @@ -104,4 +104,4 @@ jobs: --image-ids imageTag=${GITHUB_SHA} \ --query 'imageDetails[0].imageDigest' \ --output text) - echo "image_digest=$DIGEST" >> "$GITHUB_OUTPUT" + echo "image_digest=$DIGEST" >> "$GITHUB_OUTPUT" \ No newline at end of file diff --git a/.github/workflows/_deploy_lambda.yml b/.github/workflows/_deploy_lambda.yml index bff106c5..8424f0d5 100644 --- a/.github/workflows/_deploy_lambda.yml +++ b/.github/workflows/_deploy_lambda.yml @@ -23,6 +23,18 @@ on: required: true type: string + terraform_apply: + required: false + type: string + default: 'false' + # can only be 'true' or 'false' + + terraform_destroy: + required: false + type: string + default: 'false' + # can only be 'true' or 'false' + secrets: AWS_ACCESS_KEY_ID: required: true @@ -87,5 +99,11 @@ jobs: -out=lambdaplan - name: Terraform Apply + if: (inputs.terraform_apply == 'true' || github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main') && inputs.terraform_destroy != 'true' working-directory: ${{ inputs.lambda_path }} run: terraform apply -auto-approve lambdaplan + + - name: Terraform Destroy + if: inputs.terraform_destroy == 'true' + working-directory: ${{ inputs.lambda_path }} + run: terraform destroy -auto-approve \ No newline at end of file diff --git a/.github/workflows/deploy_terraform.yml b/.github/workflows/deploy_terraform.yml index 4ac08e41..6fc38976 100644 --- a/.github/workflows/deploy_terraform.yml +++ b/.github/workflows/deploy_terraform.yml @@ -4,27 +4,39 @@ on: push: branches: - "**" + paths: + - 'infrastructure/terraform/**' + - '.github/workflows/deploy_terraform.yml' + - '.github/workflows/_build_image.yml' + - '.github/workflows/_deploy_lambda.yml' jobs: determine_stage: runs-on: ubuntu-latest + outputs: stage: ${{ steps.set-stage.outputs.stage }} + env: + AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} + AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} + AWS_REGION: ${{ secrets.DEV_AWS_REGION }} + DEV_DB_HOST: ${{ secrets.DEV_DB_HOST }} + steps: - name: Determine stage from branch id: set-stage shell: bash run: | - env + echo "AWS_ACCESS_KEY_ID is set? ${AWS_ACCESS_KEY_ID:+yes}" + echo "AWS_SECRET_ACCESS_KEY is set? ${AWS_SECRET_ACCESS_KEY:+yes}" + echo "AWS_REGION=$AWS_REGION" + echo "DEV_DB_HOST=$DEV_DB_HOST" + BRANCH="${GITHUB_REF_NAME}" if [[ "$BRANCH" == "prod" ]]; then echo "stage=prod" >> "$GITHUB_OUTPUT" - - elif [[ "$BRANCH" == "dev" ]]; then - echo "stage=dev" >> "$GITHUB_OUTPUT" - else echo "stage=dev" >> "$GITHUB_OUTPUT" fi @@ -109,10 +121,17 @@ jobs: ecr_repo: postcode_splitter-${{ needs.determine_stage.outputs.stage }} dockerfile_path: backend/postcode_splitter/handler/Dockerfile build_context: . + build_args: | + DEV_DB_HOST=$DEV_DB_HOST + DEV_DB_PORT=$DEV_DB_PORT + DEV_DB_NAME=$DEV_DB_NAME secrets: AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} AWS_REGION: ${{ secrets.DEV_AWS_REGION }} + DEV_DB_HOST: ${{ secrets.DEV_DB_HOST }} + DEV_DB_PORT: ${{ secrets.DEV_DB_PORT }} + DEV_DB_NAME: ${{ secrets.DEV_DB_NAME }} # ============================================================ # 3️⃣ Deploy Postcode Splitter Lambda @@ -168,5 +187,4 @@ jobs: secrets: AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} - AWS_REGION: ${{ secrets.DEV_AWS_REGION }} - + AWS_REGION: ${{ secrets.DEV_AWS_REGION }} \ No newline at end of file From 5f7e85d3a93fc93037c98640ec0aa0fbc79e7c51 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Tue, 10 Feb 2026 16:46:26 +0000 Subject: [PATCH 04/13] safe --- .github/workflows/unit_tests.yml | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 14d5a06f..cc6431b8 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -4,9 +4,6 @@ on: pull_request: branches: - "**" - push: - branches: - - "**" jobs: @@ -30,4 +27,4 @@ jobs: env: EPC_AUTH_TOKEN: ${{ secrets.DEV_EPC_AUTH_TOKEN }} run: | - make test + make test \ No newline at end of file From 362120ba2c08aefb7e8dbddff753d7aff76dc058 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Tue, 10 Feb 2026 16:53:02 +0000 Subject: [PATCH 05/13] simpler logic --- .github/workflows/_deploy_lambda.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/_deploy_lambda.yml b/.github/workflows/_deploy_lambda.yml index 8424f0d5..3612ab43 100644 --- a/.github/workflows/_deploy_lambda.yml +++ b/.github/workflows/_deploy_lambda.yml @@ -99,11 +99,11 @@ jobs: -out=lambdaplan - name: Terraform Apply - if: (inputs.terraform_apply == 'true' || github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main') && inputs.terraform_destroy != 'true' + if: inputs.terraform_apply == 'true' && inputs.terraform_destroy != 'true' working-directory: ${{ inputs.lambda_path }} run: terraform apply -auto-approve lambdaplan - name: Terraform Destroy - if: inputs.terraform_destroy == 'true' + if: inputs.terraform_destroy == 'true' && inputs.terraform_apply != 'true' working-directory: ${{ inputs.lambda_path }} run: terraform destroy -auto-approve \ No newline at end of file From 3d7b6d65843db6ab5a3d52e1f5b711e5f9df5952 Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Tue, 10 Feb 2026 17:02:17 +0000 Subject: [PATCH 06/13] only true for dev --- .github/workflows/deploy_terraform.yml | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.github/workflows/deploy_terraform.yml b/.github/workflows/deploy_terraform.yml index 6fc38976..df378b85 100644 --- a/.github/workflows/deploy_terraform.yml +++ b/.github/workflows/deploy_terraform.yml @@ -16,6 +16,7 @@ jobs: outputs: stage: ${{ steps.set-stage.outputs.stage }} + terraform_apply: ${{ steps.set-stage.outputs.terraform_apply }} env: AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} @@ -28,17 +29,17 @@ jobs: id: set-stage shell: bash run: | - echo "AWS_ACCESS_KEY_ID is set? ${AWS_ACCESS_KEY_ID:+yes}" - echo "AWS_SECRET_ACCESS_KEY is set? ${AWS_SECRET_ACCESS_KEY:+yes}" - echo "AWS_REGION=$AWS_REGION" - echo "DEV_DB_HOST=$DEV_DB_HOST" - BRANCH="${GITHUB_REF_NAME}" if [[ "$BRANCH" == "prod" ]]; then echo "stage=prod" >> "$GITHUB_OUTPUT" + echo "terraform_apply=false" >> "$GITHUB_OUTPUT" + elif [[ "$BRANCH" == "dev" ]]; then + echo "stage=dev" >> "$GITHUB_OUTPUT" + echo "terraform_apply=true" >> "$GITHUB_OUTPUT" else echo "stage=dev" >> "$GITHUB_OUTPUT" + echo "terraform_apply=false" >> "$GITHUB_OUTPUT" fi # ============================================================ @@ -105,6 +106,7 @@ jobs: stage: ${{ needs.determine_stage.outputs.stage }} ecr_repo: address2uprn-${{ needs.determine_stage.outputs.stage }} image_digest: ${{ needs.address2uprn_image.outputs.image_digest }} + terraform_apply: ${{ needs.determine_stage.outputs.terraform_apply }} secrets: AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} @@ -145,6 +147,7 @@ jobs: stage: ${{ needs.determine_stage.outputs.stage }} ecr_repo: postcode_splitter-${{ needs.determine_stage.outputs.stage }} image_digest: ${{ needs.postcodeSplitter_image.outputs.image_digest }} + terraform_apply: ${{ needs.determine_stage.outputs.terraform_apply }} secrets: AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} @@ -184,6 +187,7 @@ jobs: stage: ${{ needs.determine_stage.outputs.stage }} ecr_repo: condition-etl-${{ needs.determine_stage.outputs.stage }} image_digest: ${{ needs.condition_etl_image.outputs.image_digest }} + terraform_apply: ${{ needs.determine_stage.outputs.terraform_apply }} secrets: AWS_ACCESS_KEY_ID: ${{ secrets.DEV_AWS_ACCESS_KEY_ID }} AWS_SECRET_ACCESS_KEY: ${{ secrets.DEV_AWS_SECRET_ACCESS_KEY }} From 9a8cc902d0c4717ed9d625a7dc9338d97e8bb6bc Mon Sep 17 00:00:00 2001 From: Jun-te Kim Date: Tue, 10 Feb 2026 17:02:40 +0000 Subject: [PATCH 07/13] comments to make it more clear --- .github/workflows/deploy_terraform.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/deploy_terraform.yml b/.github/workflows/deploy_terraform.yml index df378b85..71e2ad9d 100644 --- a/.github/workflows/deploy_terraform.yml +++ b/.github/workflows/deploy_terraform.yml @@ -38,6 +38,7 @@ jobs: echo "stage=dev" >> "$GITHUB_OUTPUT" echo "terraform_apply=true" >> "$GITHUB_OUTPUT" else + # Feature branch echo "stage=dev" >> "$GITHUB_OUTPUT" echo "terraform_apply=false" >> "$GITHUB_OUTPUT" fi From 866166c022cc703d5026cff255d0b264b58d893a Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 11 Feb 2026 12:41:37 +0000 Subject: [PATCH 08/13] handle typing error for addreses class --- .idea/inspectionProfiles/profiles_settings.xml | 1 + asset_list/app.py | 18 +++++++++--------- asset_list/mappings/roof.py | 14 +++++++++++++- asset_list/mappings/walls.py | 8 +++++++- asset_list/utils.py | 6 +++--- backend/addresses/Addresses.py | 4 ++++ 6 files changed, 37 insertions(+), 14 deletions(-) diff --git a/.idea/inspectionProfiles/profiles_settings.xml b/.idea/inspectionProfiles/profiles_settings.xml index 105ce2da..dd4c951e 100644 --- a/.idea/inspectionProfiles/profiles_settings.xml +++ b/.idea/inspectionProfiles/profiles_settings.xml @@ -1,5 +1,6 @@ + diff --git a/asset_list/app.py b/asset_list/app.py index 30172121..13a6a025 100644 --- a/asset_list/app.py +++ b/asset_list/app.py @@ -69,24 +69,24 @@ def app(): Property UPRN """ - data_folder = "/Users/khalimconn-kowlessar/Documents/hestia/Customers/Aspire" - data_filename = "ASPIRE ASSET LIST.xlsx" - sheet_name = "Asset List" - postcode_column = "Postcode" + data_folder = "/Users/khalimconn-kowlessar/Documents/hestia/Customers/West Kent" + data_filename = "West Kent Asset List.xlsx" + sheet_name = "Sheet1" + postcode_column = "POSTCODE" address1_column = None address1_method = "house_number_extraction" - fulladdress_column = "Address" + fulladdress_column = "ADDRESS" address_cols_to_concat = [] missing_postcodes_method = None landlord_year_built = None landlord_os_uprn = None - landlord_property_type = "Property Type" + landlord_property_type = "PROPERTY TYPE" landlord_built_form = None - landlord_wall_construction = None - landlord_roof_construction = None + landlord_wall_construction = "wall combined" + landlord_roof_construction = "HEATING SYSTEM" landlord_heating_system = None landlord_existing_pv = None - landlord_property_id = "LLUPRN" + landlord_property_id = "UPRN" landlord_sap = None outcomes_filename = None outcomes_sheetname = None diff --git a/asset_list/mappings/roof.py b/asset_list/mappings/roof.py index cf829a5f..70cc8742 100644 --- a/asset_list/mappings/roof.py +++ b/asset_list/mappings/roof.py @@ -308,6 +308,18 @@ ROOF_CONSTRUCTION_MAPPINGS = { 'Flat: No Insulation': 'flat uninsulated', 'AnotherDwellingAbove: Unknown, PitchedNormalLoftAccess: 250mm': 'another dwelling above', 'PitchedNormalLoftAccess: 175mm': 'pitched insulated', - 'AnotherDwellingAbove: 300mm': 'another dwelling above' + 'AnotherDwellingAbove: 300mm': 'another dwelling above', + + 'Another dwelling above, As built': 'another dwelling above', + 'Pitched (slates or tiles) no loft access, 400mm+': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 400mm+': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 300mm': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 75mm': 'pitched less than 100mm insulation', + 'Pitched (slates or tiles) no loft access, 300mm': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 270mm': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 100mm': 'pitched insulated', + 'Pitched (slates or tiles) no loft access, 200mm': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 200mm': 'pitched insulated', + 'Pitched (slates or tiles) access to loft, 50mm': 'pitched less than 100mm insulation' } diff --git a/asset_list/mappings/walls.py b/asset_list/mappings/walls.py index 1bb02a9a..1a252b33 100644 --- a/asset_list/mappings/walls.py +++ b/asset_list/mappings/walls.py @@ -363,6 +363,12 @@ WALL_CONSTRUCTION_MAPPINGS = { 'Timber Frame, As Built': 'timber frame unknown insulation', 'Solid Brick, Internal Insulation': 'insulated solid brick', 'Granite or Whinstone, As Built': 'granite or whinstone unknown insulation', - 'Solid Brick, External': 'insulated solid brick' + 'Solid Brick, External': 'insulated solid brick', + + 'Cavity, Filled cavity': 'filled cavity', + 'Solid Brick, As built': 'solid brick unknown insulation', + 'System built, As built': 'system built unknown insulation', + 'Timber frame, As built': 'timber frame unknown insulation', + 'Cavity, As built': 'cavity unknown insulation' } diff --git a/asset_list/utils.py b/asset_list/utils.py index 8746c03a..d83a35f2 100644 --- a/asset_list/utils.py +++ b/asset_list/utils.py @@ -220,7 +220,7 @@ def get_data( searcher.find_property(skip_os=True) # Check if we have a flat or appartment - if searcher.newest_epc is None and uprn is None: + if not searcher.newest_epc and uprn is None: # Try again: if SearchEpc.get_house_number(address=str(house_number), postcode=postcode) is None: # Backup @@ -252,12 +252,12 @@ def get_data( searcher.find_property(skip_os=True) # As a final resort, we estimate the EPC - if property_type is not None and searcher.newest_epc is None: + if property_type is not None and not searcher.newest_epc: searcher.ordnance_survey_client.property_type = property_type searcher.ordnance_survey_client.built_form = built_form searcher.find_property(skip_os=True) - if searcher.newest_epc is None: + if not searcher.newest_epc: no_epc.append(home[row_id_name]) continue diff --git a/backend/addresses/Addresses.py b/backend/addresses/Addresses.py index 22822c6b..e81fef50 100644 --- a/backend/addresses/Addresses.py +++ b/backend/addresses/Addresses.py @@ -1,3 +1,4 @@ +from typing import Iterator from backend.addresses.Address import Address @@ -12,6 +13,9 @@ class Addresses: def __len__(self) -> int: return len(self._addresses) + def __iter__(self) -> Iterator[Address]: + return iter(self._addresses) + @classmethod def from_plan_input(cls, plan_input: list[dict], body) -> "Addresses": addresses = [] From 9e6ff3e48d0c28dc5ae5145f9b043b6ddb825d05 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 11 Feb 2026 13:57:56 +0000 Subject: [PATCH 09/13] map empty description to null outputs for Roof Attributes and updated tests. Removed redundant repeat test --- etl/epc_clean/epc_attributes/RoofAttributes.py | 6 ++++++ etl/epc_clean/tests/test_roof_attributes.py | 13 ++----------- recommendations/RoofRecommendations.py | 6 +++--- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/etl/epc_clean/epc_attributes/RoofAttributes.py b/etl/epc_clean/epc_attributes/RoofAttributes.py index 98998e5a..7e2aed2c 100644 --- a/etl/epc_clean/epc_attributes/RoofAttributes.py +++ b/etl/epc_clean/epc_attributes/RoofAttributes.py @@ -74,6 +74,8 @@ class RoofAttributes(Definitions): "insulation_thickness", ] + NODATA_NULLS = ["insulation_thickness", "thermal_transmittance", "thermal_transmittance_unit"] + def __init__(self, description: str): """ :param description: Description of the roof. @@ -153,6 +155,10 @@ class RoofAttributes(Definitions): if self.nodata: for key in self.DEFAULT_KEYS: result[key] = False + # Insulation thickness, thermal transmittance and thermal transmittance unit are set to None for nodata + # cases + for k in self.NODATA_NULLS: + result[k] = None return result description = self.description diff --git a/etl/epc_clean/tests/test_roof_attributes.py b/etl/epc_clean/tests/test_roof_attributes.py index 33c6a829..d0d277c7 100644 --- a/etl/epc_clean/tests/test_roof_attributes.py +++ b/etl/epc_clean/tests/test_roof_attributes.py @@ -26,9 +26,9 @@ class TestRoofAttributes: def test_empty_str(self): # Test initialization with an empty description assert RoofAttributes('').process() == { - 'thermal_transmittance': False, 'thermal_transmittance_unit': False, 'is_pitched': False, + 'thermal_transmittance': None, 'thermal_transmittance_unit': None, 'is_pitched': False, 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, 'is_at_rafters': False, - 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': False, 'insulation_thickness': False + 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': False, 'insulation_thickness': None } assert set(list(RoofAttributes('').process().values())) == {False} @@ -92,15 +92,6 @@ class TestRoofAttributes: with pytest.raises(ValueError): RoofAttributes('nonsense string').process() - def test_clean_roof_no_description(self): - roof = RoofAttributes('').process() - assert roof == { - 'thermal_transmittance': False, 'thermal_transmittance_unit': False, 'is_pitched': False, - 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, - 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': False, - 'insulation_thickness': False - } - def test_clean_roof_edge_cases(self): # Insulation thickness edge case assert RoofAttributes('Pitched, 99999 mm loft insulation').process()['insulation_thickness'] == "99999" diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index f88a672b..5e118844 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -59,9 +59,9 @@ class RoofRecommendations: # Extract the insulation thickness from the roof, which is used throughout this method self.insulation_thickness = convert_thickness_to_numeric( - self.property.roof["insulation_thickness"], - self.property.roof["is_pitched"], - self.property.roof["is_flat"] + string_thickness=self.property.roof["insulation_thickness"], + is_pitched=self.property.roof["is_pitched"], + is_flat=self.property.roof["is_flat"] ) @classmethod From 7ad2b3d46b52235cee131efd5f05c3dc93ed94fd Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 11 Feb 2026 16:53:19 +0000 Subject: [PATCH 10/13] corrected test --- etl/epc_clean/tests/test_roof_attributes.py | 1 - 1 file changed, 1 deletion(-) diff --git a/etl/epc_clean/tests/test_roof_attributes.py b/etl/epc_clean/tests/test_roof_attributes.py index d0d277c7..e400f95a 100644 --- a/etl/epc_clean/tests/test_roof_attributes.py +++ b/etl/epc_clean/tests/test_roof_attributes.py @@ -30,7 +30,6 @@ class TestRoofAttributes: 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, 'is_at_rafters': False, 'is_assumed': False, 'has_dwelling_above': False, 'is_valid': False, 'insulation_thickness': None } - assert set(list(RoofAttributes('').process().values())) == {False} def test_clean_roof(self): result = RoofAttributes('Pitched, 270 mm loft insulation').process() From d3a494a7e7902baa6ed5b0d07af6d35ce27654bf Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Wed, 11 Feb 2026 17:09:08 +0000 Subject: [PATCH 11/13] handling missing roof description --- recommendations/RoofRecommendations.py | 4 ++++ .../tests/test_roof_recommendations.py | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 5e118844..0021edcc 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -300,6 +300,10 @@ class RoofRecommendations: ): return False + if self.property.roof["original_description"] is None: + # There is no description so we cannot make an assessment + return False + return True @staticmethod diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index 0879757f..64a4d9d6 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -8,6 +8,30 @@ from recommendations.tests.test_data.materials import materials class TestRoofRecommendations: + def test_null_roof_description(self): + epc_record = EPCRecord() + epc_record.prepared_epc = { + "county": "Cambridgeshire", + } + property_instance = Property(id=0, address="fake", postcode="fake", epc_record=epc_record) + property_instance.age_band = "F" + property_instance.insulation_floor_area = 100 + property_instance.roof = { + 'original_description': None, + 'clean_description': None, + 'thermal_transmittance': None, + 'thermal_transmittance_unit': None, + 'is_pitched': True, 'is_roof_room': False, 'is_loft': False, 'is_flat': False, 'is_thatched': False, + 'is_at_rafters': False, 'is_assumed': True, 'has_dwelling_above': False, 'is_valid': True, + 'insulation_thickness': 'none', 'roof_thermal_transmittance': None, 'roof_insulation_thickness': None + } + property_instance.already_installed = [] + + roof_recommender = RoofRecommendations(property_instance=property_instance, materials=materials) + roof_recommender.recommend(phase=0) + + assert not roof_recommender.recommendations + def test_loft_insulation_recommendation_no_insulation(self): epc_record = EPCRecord() epc_record.prepared_epc = { From 24a7c95f638dad55690a6c5766c534b7aab17f24 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 13 Feb 2026 10:42:17 +0000 Subject: [PATCH 12/13] handling cases where ventilation recommendation has a score taking a property below 1 --- recommendations/Recommendations.py | 18 +++ recommendations/tests/test_recommendations.py | 126 ++++++++++++++++-- 2 files changed, 131 insertions(+), 13 deletions(-) diff --git a/recommendations/Recommendations.py b/recommendations/Recommendations.py index e470c1a3..acd49e05 100644 --- a/recommendations/Recommendations.py +++ b/recommendations/Recommendations.py @@ -768,6 +768,24 @@ class Recommendations: # Update the current phase values current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"] + + # This is very much an edge case but we also the end result taking the property + # below a SAP rating of 1, which is the minimum SAP rating + if previous_phase_values["sap"] + property_phase_impact["sap"] < 1: + sap_adjustment = 1 - (previous_phase_values["sap"] + property_phase_impact["sap"]) + adjustments.append( + { + "recommendation_id": rec["recommendation_id"], + "phase": rec["phase"], + "sap_adjustment": sap_adjustment, + } + ) + # The new impact should be the current impact plus the adjustment + property_phase_impact["sap"] = property_phase_impact["sap"] + sap_adjustment + + # Update the current phase values + current_phase_values["sap"] = previous_phase_values["sap"] + property_phase_impact["sap"] + elif rec["type"] == "loft_insulation": # When we have a loft insulation recommendation, where there is an extension and the existing # amount of loft insulation is already good, we limit the SAP points diff --git a/recommendations/tests/test_recommendations.py b/recommendations/tests/test_recommendations.py index a9915422..e3bcbb2f 100644 --- a/recommendations/tests/test_recommendations.py +++ b/recommendations/tests/test_recommendations.py @@ -347,21 +347,21 @@ def property_instance(): "input_data, expected", [ ( - [ - {"recommendation_id": "a", "phase": 0, "sap_adjustment": 1.7}, - {"recommendation_id": "b", "phase": 0, "sap_adjustment": 1.7}, - ], - [{"recommendation_id": "a", "phase": 0, "sap_adjustment": 1.7}], + [ + {"recommendation_id": "a", "phase": 0, "sap_adjustment": 1.7}, + {"recommendation_id": "b", "phase": 0, "sap_adjustment": 1.7}, + ], + [{"recommendation_id": "a", "phase": 0, "sap_adjustment": 1.7}], ), ( - [ - {"recommendation_id": "a", "phase": 1, "sap_adjustment": 2}, - {"recommendation_id": "b", "phase": 2, "sap_adjustment": 3}, - ], - [ - {"recommendation_id": "a", "phase": 1, "sap_adjustment": 2}, - {"recommendation_id": "b", "phase": 2, "sap_adjustment": 3}, - ], + [ + {"recommendation_id": "a", "phase": 1, "sap_adjustment": 2}, + {"recommendation_id": "b", "phase": 2, "sap_adjustment": 3}, + ], + [ + {"recommendation_id": "a", "phase": 1, "sap_adjustment": 2}, + {"recommendation_id": "b", "phase": 2, "sap_adjustment": 3}, + ], ), ], ) @@ -1478,3 +1478,103 @@ def test_lighting_and_loft_adjustment_combined(property_instance, heat_demand_pr {'recommendation_id': '0_phase=0', 'phase': 0, 'sap_adjustment': np.float64(1.7)}, {'recommendation_id': '4_phase=2', 'phase': 2, 'sap_adjustment': np.float64(4.0)} ] + + +def test_mechanical_ventilation_sap_floor(property_instance): + rec = { + "type": "mechanical_ventilation", + "recommendation_id": "mv_test", + "phase": 1, + } + + previous_phase_values = {"sap": 2.0} + current_phase_values = {"sap": 0.5} # model prediction already below 1 + property_phase_impact = {"sap": -1.5, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec=rec, + property_phase_impact=property_phase_impact, + previous_phase_values=previous_phase_values, + current_phase_values=current_phase_values, + adjustments=adjustments, + property_instance=property_instance + ) + ) + + # SAP should be clamped to minimum 1 + assert updated_current["sap"] == 1.0 + + # Original final SAP would have been 0.5 → so adjustment = 1 - 0.5 = 0.5 + assert updated_adjustments == [ + { + "recommendation_id": "mv_test", + "phase": 1, + "sap_adjustment": 0.5, + } + ] + + # Impact should now reflect new clamped SAP + assert updated_impact["sap"] == -1.0 # 2.0 → 1.0 + + +def test_mechanical_ventilation_no_floor_adjustment(property_instance): + rec = { + "type": "mechanical_ventilation", + "recommendation_id": "mv_test", + "phase": 1, + } + + previous_phase_values = {"sap": 5.0} + current_phase_values = {"sap": 3.0} + property_phase_impact = {"sap": -2.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec=rec, + property_phase_impact=property_phase_impact, + previous_phase_values=previous_phase_values, + current_phase_values=current_phase_values, + adjustments=adjustments, + property_instance=property_instance + ) + ) + + # No adjustment expected + assert updated_adjustments == [] + + # SAP unchanged + assert updated_current["sap"] == 3.0 + assert updated_impact["sap"] == -2.0 + + +def test_mechanical_ventilation_exactly_one_no_adjustment(property_instance): + # Test when SAP = 1 + rec = { + "type": "mechanical_ventilation", + "recommendation_id": "mv_test", + "phase": 1, + } + + previous_phase_values = {"sap": 2.0} + current_phase_values = {"sap": 1.0} + property_phase_impact = {"sap": -1.0, "carbon": 0, "heat_demand": 0} + adjustments = [] + + updated_impact, updated_current, updated_adjustments = ( + Recommendations._apply_measure_specific_rules( + rec=rec, + property_phase_impact=property_phase_impact, + previous_phase_values=previous_phase_values, + current_phase_values=current_phase_values, + adjustments=adjustments, + property_instance=property_instance + ) + ) + + # Exactly 1 → no adjustment + assert updated_adjustments == [] + assert updated_current["sap"] == 1.0 + assert updated_impact["sap"] == -1.0 From 7237ad5a7969d099b804f2004be41f1722c5e641 Mon Sep 17 00:00:00 2001 From: Khalim Conn-Kowlessar Date: Fri, 13 Feb 2026 12:40:07 +0000 Subject: [PATCH 13/13] bug fixed for 0 target gain where infinity was being selected --- backend/engine/engine.py | 2 +- .../optimiser/funding_optimiser.py | 41 +++-- recommendations/tests/test_optimisers.py | 141 ++++++++++++++++++ sfr/principal_pitch/2_export_data.py | 37 +---- 4 files changed, 172 insertions(+), 49 deletions(-) diff --git a/backend/engine/engine.py b/backend/engine/engine.py index 69726604..80d6d078 100644 --- a/backend/engine/engine.py +++ b/backend/engine/engine.py @@ -865,7 +865,7 @@ async def model_engine(body: PlanTriggerRequest): check_duplicate_property_ids(input_properties) logger.info("Inserting property data") - # We now bulk upload all of the EPC data + # We now bulk upload all the EPC data with db_session() as session: db_funcs.epc_functions.EpcStoreService.bulk_upsert_epc_data(session, epc_upserts) diff --git a/recommendations/optimiser/funding_optimiser.py b/recommendations/optimiser/funding_optimiser.py index a2f138ed..6afe7d78 100644 --- a/recommendations/optimiser/funding_optimiser.py +++ b/recommendations/optimiser/funding_optimiser.py @@ -10,6 +10,7 @@ In the future, we will adapt this into a class-based structure to allow for more from copy import deepcopy import pandas as pd import numpy as np +from typing import Mapping, Union from itertools import product from backend.app.plan.schemas import ( @@ -823,21 +824,23 @@ def optimise_with_scenarios( # No special path; just exclude ASHP from options and allow us to optimise. measures_no_heat_pump = exclude_measure_types(optimisation_measures, ["air_source_heat_pump"]) - picked, total_cost, total_gain = run_optimizer( - measures_no_heat_pump, - budget=budget, - sub_target_gain=target_gain, - ) + if target_gain > 0: + # If we don't have any gain, we don't actually need to do this + picked, total_cost, total_gain = run_optimizer( + measures_no_heat_pump, + budget=budget, + sub_target_gain=target_gain, + ) - if picked is not None: - solutions.append({ - "scenario": "no_heat_pump", - "items": picked, - "fixed_items": [], - "total_cost": total_cost, - "total_gain": total_gain, - "already_installed_gain": sum([x["gain"] for x in picked if x["already_installed"]]) - }) + if picked is not None: + solutions.append({ + "scenario": "no_heat_pump", + "items": picked, + "fixed_items": [], + "total_cost": total_cost, + "total_gain": total_gain, + "already_installed_gain": sum([x["gain"] for x in picked if x["already_installed"]]) + }) solutions_df = append_solution_metrics(solutions, target_gain, p, already_installed_sap) @@ -1101,7 +1104,12 @@ def contributes_min_insulation(opt_types): }) -def run_optimizer(input_measures, budget=None, sub_target_gain=None, allow_slack=False): +def run_optimizer( + input_measures: list[list[Mapping[str, int | float | str]]], + budget: Union[float, None] = None, + sub_target_gain: Union[float, None] = None, + allow_slack: bool = False +): """ Thin wrapper over your optimisers. Returns: list[dict] selected_options @@ -1112,7 +1120,7 @@ def run_optimizer(input_measures, budget=None, sub_target_gain=None, allow_slack if budget is not None: opt = GainOptimiser( - input_measures, max_cost=budget, max_gain=(sub_target_gain or float("inf")), + input_measures, max_cost=budget, max_gain=0 if sub_target_gain == 0 else (sub_target_gain or float("inf")), allow_slack=allow_slack ) else: @@ -1123,6 +1131,7 @@ def run_optimizer(input_measures, budget=None, sub_target_gain=None, allow_slack opt.setup() opt.solve() cost = sum([x["cost"] for x in opt.solution]) + return opt.solution, cost, opt.solution_gain diff --git a/recommendations/tests/test_optimisers.py b/recommendations/tests/test_optimisers.py index 17e45154..0c794119 100644 --- a/recommendations/tests/test_optimisers.py +++ b/recommendations/tests/test_optimisers.py @@ -1,6 +1,7 @@ import pytest from recommendations.optimiser.funding_optimiser import build_heat_pump_paths +from recommendations.optimiser.funding_optimiser import run_optimizer class DummyProp: @@ -68,3 +69,143 @@ def test_build_heat_pump_paths(): assert eg2 == [{'AND': ['internal_wall_insulation', 'loft_insulation', 'air_source_heat_pump']}, {'AND': ['external_wall_insulation', 'loft_insulation', 'air_source_heat_pump']}] + + +def test_run_optimizer_empty_input(): + solution, cost, gain = run_optimizer([]) + assert solution is None + assert cost == 0.0 + assert gain == 0.0 + + +def test_uses_gain_optimiser_when_budget_provided(monkeypatch): + captured_args = {} + + class FakeGainOptimiser: + def __init__(self, measures, max_cost, max_gain, allow_slack): + captured_args["measures"] = measures + captured_args["max_cost"] = max_cost + captured_args["max_gain"] = max_gain + captured_args["allow_slack"] = allow_slack + self.solution = [{"cost": 100}] + self.solution_gain = 5 + + def setup(self): + pass + + def solve(self): + pass + + monkeypatch.setattr( + "recommendations.optimiser.funding_optimiser.GainOptimiser", + FakeGainOptimiser + ) + + measures = [[{"cost": 100, "gain": 5}]] + + solution, cost, gain = run_optimizer( + measures, + budget=500, + sub_target_gain=10, + allow_slack=True + ) + + assert captured_args["max_cost"] == 500 + assert captured_args["max_gain"] == 10 + assert captured_args["allow_slack"] is True + assert cost == 100 + assert gain == 5 + + +def test_sub_target_gain_zero_sets_max_gain_zero(monkeypatch): + captured_args = {} + + class FakeGainOptimiser: + def __init__(self, measures, max_cost, max_gain, allow_slack): + captured_args["max_gain"] = max_gain + self.solution = [] + self.solution_gain = 0 + + def setup(self): + pass + + def solve(self): + pass + + monkeypatch.setattr( + "recommendations.optimiser.funding_optimiser.GainOptimiser", + FakeGainOptimiser + ) + + measures = [[{"cost": 100, "gain": 5}]] + + run_optimizer( + measures, + budget=500, + sub_target_gain=0 + ) + + assert captured_args["max_gain"] == 0 + + +def test_sub_target_gain_none_sets_max_gain_infinity(monkeypatch): + captured_args = {} + + class FakeGainOptimiser: + def __init__(self, measures, max_cost, max_gain, allow_slack): + captured_args["max_gain"] = max_gain + self.solution = [] + self.solution_gain = 0 + + def setup(self): + pass + + def solve(self): + pass + + monkeypatch.setattr( + "recommendations.optimiser.funding_optimiser.GainOptimiser", + FakeGainOptimiser + ) + + measures = [[{"cost": 100, "gain": 5}]] + + run_optimizer( + measures, + budget=500, + sub_target_gain=None + ) + + assert captured_args["max_gain"] == float("inf") + + +def test_uses_cost_optimiser_when_no_budget(monkeypatch): + captured_args = {} + + class FakeCostOptimiser: + def __init__(self, measures, min_gain): + captured_args["min_gain"] = min_gain + self.solution = [{"cost": 50}] + self.solution_gain = 10 + + def setup(self): + pass + + def solve(self): + pass + + monkeypatch.setattr( + "recommendations.optimiser.funding_optimiser.CostOptimiser", + FakeCostOptimiser + ) + + measures = [[{"cost": 50, "gain": 10}]] + + solution, cost, gain = run_optimizer( + measures, + sub_target_gain=10 + ) + + assert captured_args["min_gain"] == 10 + assert cost == 50 + assert gain == 10 diff --git a/sfr/principal_pitch/2_export_data.py b/sfr/principal_pitch/2_export_data.py index a65509d5..b62e51d7 100644 --- a/sfr/principal_pitch/2_export_data.py +++ b/sfr/principal_pitch/2_export_data.py @@ -28,12 +28,12 @@ from sqlalchemy import func # PORTFOLIO_ID = 206 # SCENARIOS = [389] -PORTFOLIO_ID = 524 +PORTFOLIO_ID = 568 SCENARIOS = [ - 1009, + 1059, ] scenario_names = { - 1009: "EPC C; Most Economic", + 1059: "EPC C - 10k budget", } @@ -230,7 +230,7 @@ for scenario_id in SCENARIOS: # Get recs for this scenario recommended_measures_df = recommendations_df[ recommendations_df["scenario_id"] == scenario_id - ][["property_id", "measure_type", "estimated_cost", "default"]] + ][["property_id", "measure_type", "estimated_cost", "default"]] recommended_measures_df = recommended_measures_df[ recommended_measures_df["default"] ] @@ -238,7 +238,7 @@ for scenario_id in SCENARIOS: post_install_sap = recommendations_df[ recommendations_df["scenario_id"] == scenario_id - ][["property_id", "default", "sap_points"]] + ][["property_id", "default", "sap_points"]] post_install_sap = post_install_sap[post_install_sap["default"]] # Sum up the sap points by property id post_install_sap = ( @@ -301,33 +301,6 @@ for scenario_id in SCENARIOS: ) df["uprn"] = df["uprn"].astype(str) - relevant_plans = plans_df[plans_df["scenario_id"] == scenario_id] - df2 = df.merge( - relevant_plans[["property_id", "post_sap_points", "post_epc_rating"]], - how="left", - on="property_id", - suffixes=("", "_plan"), - ) - print(df2["predicted_post_works_epc"].value_counts()) - print(df2["post_epc_rating"].value_counts()) - - z = df2[ - (df2["predicted_post_works_epc"] != "D") - & (df2["post_epc_rating"].astype(str) == "Epc.D") - ] - - df2["predicted_post_works_epc"].value_counts() - df2["post_epc_rating"].astype(str).value_counts() - - df2[df2["total_retrofit_cost"] > 0].shape - - getting_works = df[df["total_retrofit_cost"] > 0] - getting_works["predicted_post_works_epc"].value_counts() - - 32565 / getting_works.shape[0] - - df[df["predicted_post_works_sap"] == ""] - # Create excel to store to filename = f"{scenario_names[scenario_id]} - 20250113 final.xlsx" with pd.ExcelWriter(filename) as writer: