diff --git a/recommendations/RoofRecommendations.py b/recommendations/RoofRecommendations.py index 08fceb32..5eb81439 100644 --- a/recommendations/RoofRecommendations.py +++ b/recommendations/RoofRecommendations.py @@ -122,10 +122,13 @@ class RoofRecommendations: @staticmethod def is_sloping_ceiling_appropriate( - is_pitched: bool, is_loft: bool, is_assumed: bool, + is_pitched: bool, + is_loft: bool, + is_assumed: bool, is_flat: bool, has_sloping_ceiling_recommendation: bool, - primary_roof_is_sloped: bool, insulation_thickness: str, + primary_roof_looks_sloped: bool, + insulation_thickness: str, has_loft_insulation_recommendation: bool ) -> bool: """ @@ -136,7 +139,7 @@ class RoofRecommendations: :param is_assumed: Boolean - indiates if the assessment of the roof is assumed or actually confirmed :param has_sloping_ceiling_recommendation: Boolean - indicates if the property has a sloping ceiling recommendation - :param primary_roof_is_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to + :param primary_roof_looks_sloped: Boolean - indicates if the primary room is described a sloped (as opposed to an extension) :param insulation_thickness: String - insulation thickness of the roof :param has_loft_insulation_recommendation: Boolean - indicates whether or not there @@ -151,7 +154,7 @@ class RoofRecommendations: # If we have a loft primary roof and sloping ceiling has_suitable_features = ( - is_pitched and not is_loft and not is_assumed and primary_roof_is_sloped + is_pitched and not is_loft and not is_assumed and primary_roof_looks_sloped ) # Check if it needs a recommendation @@ -227,7 +230,7 @@ class RoofRecommendations: @staticmethod def is_flat_roof_insulation_appropriate( - is_flat: bool, measures: List, has_flat_roof_recommendation: bool, primary_roof_is_sloped: bool + is_flat: bool, measures: List, has_flat_roof_recommendation: bool, primary_roof_looks_sloped: bool ) -> bool: """ Determine if flat roof insulation is appropriate @@ -235,17 +238,17 @@ class RoofRecommendations: :param measures: List - list of measures :param has_flat_roof_recommendation: Boolean - indicates whether or not there is a flat roof non-invasive recommendation - :param primary_roof_is_sloped: Boolean - indicates if the primary roof is flat + :param primary_roof_looks_sloped: Boolean - indicates if the primary roof looks like a sloped roof :return: Boolean - When checking if has_flat_roof_recommendation and primary_roof_is_sloped, we need to check both + When checking if has_flat_roof_recommendation and primary_roof_looks_sloped, we need to check both conditions. This is because within a default EPC recommendation, the EPC will pair these recommendations together. Therefore, weneed to ensure the primary roof isn't sloped """ flat_roof_in_measures = "flat_roof_insulation" in measures - return (is_flat and flat_roof_in_measures) or (has_flat_roof_recommendation and not primary_roof_is_sloped) + return (is_flat and flat_roof_in_measures) or (has_flat_roof_recommendation and not primary_roof_looks_sloped) @staticmethod def is_room_roof_insulation_appropriate( @@ -300,7 +303,7 @@ class RoofRecommendations: return True @staticmethod - def _is_primary_roof_sloped( + def _does_primary_roof_look_sloped( is_pitched: bool, is_loft: bool, is_assumed: bool ): """ @@ -387,6 +390,10 @@ class RoofRecommendations: # We only use this when we have sloping ceiling and loft insulation recommendations # Components are indexed from 0 + + needs_sloping = True + needs_loft = True + roof_count = max( x["appearance_index"] for x in find_my_epc_components if x["component_name"] == "Roof" ) + 1 @@ -403,12 +410,13 @@ class RoofRecommendations: if (roof_count <= 1) or not has_both_recommendations: if roof_count > 1: if "loft_insulation" in roof_non_invasive_recommendations: - return False, True + return not needs_sloping, needs_loft if "sloping_ceiling_insulation" in roof_non_invasive_recommendations: - return True, False + return needs_sloping, not needs_loft - return True, False # Indicates that the property needs sloping ceiling as we only run this in that case + return needs_sloping, not needs_loft # Indicates that the property needs sloping ceiling as we only run + # this in that case extracted_roof_descriptions = { idx: { @@ -432,7 +440,7 @@ class RoofRecommendations: is_loft=is_loft, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=True, + primary_roof_looks_sloped=True, insulation_thickness=insulation_thickness, has_loft_insulation_recommendation=has_loft_insulation_recommendation ) @@ -460,9 +468,9 @@ class RoofRecommendations: primary_roof_type = self._deduce_primary_roof(component_needs) if primary_roof_type in ["ambiguous", "sloping_ceiling"]: - return True, False # Set sloping ceiling to true, loft to false + return needs_sloping, not needs_loft # Set sloping ceiling to true, loft to false - return False, True # Set sloping ceiling to false, loft to true + return not needs_sloping, needs_loft # Set sloping ceiling to false, loft to true def recommend(self, phase: int, measures: List | None = None, default_u_values: bool = False): """ @@ -479,6 +487,10 @@ class RoofRecommendations: property_needs_roof_recommendation = self._does_roof_need_recommendation(measures, u_value) if not property_needs_roof_recommendation: + # Roof is either: + # - already sufficiently insulated + # - unsuitable (dwelling above, thatched, etc.) + # - not matching available measures return u_value = get_roof_u_value( @@ -517,13 +529,14 @@ class RoofRecommendations: has_loft_insulation_recommendation = any(x["type"] == "loft_insulation" for x in non_invasive_recommendations) has_flat_roof_recommendation = any(x["type"] == "flat_roof_insulation" for x in non_invasive_recommendations) has_room_roof_recommendation = any(x["type"] == "room_roof_insulation" for x in non_invasive_recommendations) - # Very naive condition - primary_roof_is_sloped = self._is_primary_roof_sloped( + + primary_roof_looks_sloped = self._does_primary_roof_look_sloped( is_pitched=is_pitched, is_loft=is_loft, is_assumed=is_assumed ) rir_over_loft = ( - is_pitched and self.property.roof["insulation_thickness"] == "none" and - "room_in_roof_insulation" in [x["type"] for x in non_invasive_recommendations] + is_pitched and + self.property.roof["insulation_thickness"] == "none" and + has_room_roof_recommendation ) needs_sloping_ceiling = self.is_sloping_ceiling_appropriate( @@ -532,7 +545,7 @@ class RoofRecommendations: is_loft=is_loft, is_assumed=is_assumed, has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped, + primary_roof_looks_sloped=primary_roof_looks_sloped, insulation_thickness=insulation_thickness, has_loft_insulation_recommendation=has_loft_insulation_recommendation ) @@ -550,7 +563,7 @@ class RoofRecommendations: is_flat=is_flat, measures=measures, has_flat_roof_recommendation=has_flat_roof_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped + primary_roof_looks_sloped=primary_roof_looks_sloped ) needs_rir_insulation = self.is_room_roof_insulation_appropriate( is_room_roof=is_room_roof, @@ -561,6 +574,9 @@ class RoofRecommendations: # We handle possible multi roof types if needs_sloping_ceiling: + # Multi-roof override: + # In ambiguous cases (extensions, mixed descriptions), EPC component analysis + # may force us to choose between loft vs sloping ceiling. needs_sloping_ceiling, needs_loft_insulation = self._handle_multi_roof_types( measures=measures, find_my_epc_components=self.property.find_my_epc_components, @@ -569,6 +585,17 @@ class RoofRecommendations: has_loft_insulation_recommendation=has_loft_insulation_recommendation, rir_over_loft=rir_over_loft ) + # Explicit override + needs_flat_roof_insulation = False + needs_rir_insulation = False + if needs_sloping_ceiling and needs_loft_insulation: + raise RuntimeError( + "Multi-roof resolution produced conflicting outcomes: " + "both sloping ceiling and loft insulation required" + ) + + # Retrofit precedence (least → most invasive): + # Loft > Flat roof > Room in roof > Sloping ceiling ################################################################ # ~~~~~ Loft Insulation Recommendation Logic ~~~~~ @@ -616,7 +643,14 @@ class RoofRecommendations: ) return - raise NotImplementedError("Implement me") + raise RuntimeError( + "Roof recommendation undecidable. " + f"needs_loft={needs_loft_insulation}, " + f"needs_flat={needs_flat_roof_insulation}, " + f"needs_rir={needs_rir_insulation}, " + f"needs_sloping={needs_sloping_ceiling}, " + f"roof={self.property.roof}" + ) @staticmethod def make_roof_insulation_description(material): diff --git a/recommendations/tests/test_roof_recommendations.py b/recommendations/tests/test_roof_recommendations.py index 9f39779c..0879757f 100644 --- a/recommendations/tests/test_roof_recommendations.py +++ b/recommendations/tests/test_roof_recommendations.py @@ -407,7 +407,7 @@ class TestRoofRecommendations: # ~~~~~~~~~~~~ Sloping Ceiling Insulation ~~~~~~~~~~~~ @pytest.mark.parametrize( - "roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, insulation_thickness, " + "roof, has_sloping_ceiling_recommendation, primary_roof_looks_sloped, insulation_thickness, " "has_loft_insulation_recommendation, expected_result", [ ( @@ -449,7 +449,7 @@ class TestRoofRecommendations: ] ) def test_is_sloping_ceiling_appropriate( - self, roof, has_sloping_ceiling_recommendation, primary_roof_is_sloped, + self, roof, has_sloping_ceiling_recommendation, primary_roof_looks_sloped, insulation_thickness, has_loft_insulation_recommendation, expected_result ): assert RoofRecommendations.is_sloping_ceiling_appropriate( @@ -458,7 +458,7 @@ class TestRoofRecommendations: is_loft=roof["is_loft"], is_assumed=roof["is_assumed"], has_sloping_ceiling_recommendation=has_sloping_ceiling_recommendation, - primary_roof_is_sloped=primary_roof_is_sloped, + primary_roof_looks_sloped=primary_roof_looks_sloped, insulation_thickness=insulation_thickness, has_loft_insulation_recommendation=has_loft_insulation_recommendation ) == expected_result