From 105358c14e5479cee580e59875b20f7ddbf84cdc Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Thu, 9 Nov 2023 14:40:30 +0100 Subject: [PATCH 1/8] building-comparison: fix checks and result description --- .../building_comparison/indicator.py | 58 ++++++++++--------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/ohsome_quality_api/indicators/building_comparison/indicator.py b/ohsome_quality_api/indicators/building_comparison/indicator.py index 870d428c4..be6319dc9 100644 --- a/ohsome_quality_api/indicators/building_comparison/indicator.py +++ b/ohsome_quality_api/indicators/building_comparison/indicator.py @@ -49,31 +49,27 @@ async def preprocess(self) -> None: else: self.coverage["EUBUCCO"] = None return - - db_query_result = await db_client.get_building_area(self.feature) - raw = db_query_result[0]["area"] or 0 - self.area_references["EUBUCCO"] = raw / (1000 * 1000) - - osm_query_result = await ohsome_client.query( - self.topic, - self.feature, - ) - raw = osm_query_result["result"][0]["value"] or 0 # if None - self.area_osm = raw / (1000 * 1000) - self.result.timestamp_osm = parser.isoparse( - osm_query_result["result"][0]["timestamp"] - ) + if not self.check_major_edge_cases(): + db_query_result = await db_client.get_building_area(self.feature) + raw = db_query_result[0]["area"] or 0 + self.area_references["EUBUCCO"] = raw / (1000 * 1000) + + osm_query_result = await ohsome_client.query( + self.topic, + self.feature, + ) + raw = osm_query_result["result"][0]["value"] or 0 # if None + self.area_osm = raw / (1000 * 1000) + self.result.timestamp_osm = parser.isoparse( + osm_query_result["result"][0]["timestamp"] + ) def calculate(self) -> None: # TODO: put checks into check_corner_cases. Let result be undefined. - - major_edge_case_description = self.check_major_edge_cases() - if major_edge_case_description: - self.result.description = major_edge_case_description + if not self.result.description == "": return - elif self.check_minor_edge_cases(): + if self.check_minor_edge_cases(): self.result.description = self.check_minor_edge_cases() - return else: self.result.description = "" @@ -127,27 +123,33 @@ def create_figure(self) -> None: raw["layout"].pop("template") # remove boilerplate self.result.figure = raw - def check_major_edge_cases(self) -> str: + def check_major_edge_cases(self) -> bool: coverage = self.coverage["EUBUCCO"] # TODO: generalize function if coverage is None or coverage == 0.00: - return "Reference dataset does not cover area-of-interest." + self.result.description = ( + "Reference dataset does not cover area-of-interest." + ) + return True elif coverage < 0.50: - return ( - "Only {:.2f}% of the area-of-interest is covered ".format(coverage) + self.result.description = ( + "Only {:.2f}% of the area-of-interest is covered ".format( + coverage * 100 + ) + "by the reference dataset (EUBUCCO). " + "No quality estimation is possible." ) + return True else: - return "" + self.result.description = "" + return False def check_minor_edge_cases(self) -> str: coverage = self.coverage["EUBUCCO"] if coverage < 0.85: return ( - "Only {:.2f}% of the area-of-interest is covered ".format(coverage) - + "by the reference dataset (EUBUCCO). " - + "No quality estimation is possible." + "Warning: Low coverage by the reference dataset (EUBUCCO). " + + "Quality estimation may be inaccurate. " ) else: return "" From 2fc250fe66ca0513b2c220f876e6ba80df536a4a Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Thu, 9 Nov 2023 15:29:11 +0100 Subject: [PATCH 2/8] building-comparison: add SQL statement for intersection of given geometry and eubucco coverage. Co-authored: Jonas Danner --- ohsome_quality_api/geodatabase/client.py | 13 +++++++++++++ .../geodatabase/get_coverage_intersection.sql | 11 +++++++++++ .../indicators/building_comparison/indicator.py | 1 + tests/integrationtests/test_geodatabase.py | 9 ++++++++- 4 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 ohsome_quality_api/geodatabase/get_coverage_intersection.sql diff --git a/ohsome_quality_api/geodatabase/client.py b/ohsome_quality_api/geodatabase/client.py index 8d6b4dab7..7842ba63e 100644 --- a/ohsome_quality_api/geodatabase/client.py +++ b/ohsome_quality_api/geodatabase/client.py @@ -17,6 +17,7 @@ from contextlib import asynccontextmanager import asyncpg +import geojson from asyncpg import Record from geojson import Feature, FeatureCollection @@ -98,3 +99,15 @@ async def get_eubucco_coverage_intersection_area(bpoly: Feature) -> list[Record] geom = str(bpoly.geometry) async with get_connection() as conn: return await conn.fetch(query, geom) + + +async def get_coverage_intersection(bpoly: Feature) -> Feature: + """Get intersection geometry of AoI and coverage geometry.""" + file_path = os.path.join(WORKING_DIR, "get_coverage_intersection.sql") + with open(file_path, "r") as file: + query = file.read() + geom = str(bpoly.geometry) + async with get_connection() as conn: + result = await conn.fetch(query, geom) + bpoly["geometry"] = geojson.loads(result[0]["geom"]) + return bpoly diff --git a/ohsome_quality_api/geodatabase/get_coverage_intersection.sql b/ohsome_quality_api/geodatabase/get_coverage_intersection.sql new file mode 100644 index 000000000..d2f49edac --- /dev/null +++ b/ohsome_quality_api/geodatabase/get_coverage_intersection.sql @@ -0,0 +1,11 @@ +WITH bpoly AS ( + SELECT + ST_Setsrid (ST_GeomFromGeoJSON ($1), 4326) AS geom +) +SELECT + ST_AsGeoJSON (ST_Intersection (bpoly.geom, coverage.geom)) AS geom +FROM + bpoly, + eubucco_v0_1_coverage_simple coverage +WHERE + ST_Intersects (bpoly.geom, coverage.geom) diff --git a/ohsome_quality_api/indicators/building_comparison/indicator.py b/ohsome_quality_api/indicators/building_comparison/indicator.py index be6319dc9..98c458004 100644 --- a/ohsome_quality_api/indicators/building_comparison/indicator.py +++ b/ohsome_quality_api/indicators/building_comparison/indicator.py @@ -50,6 +50,7 @@ async def preprocess(self) -> None: self.coverage["EUBUCCO"] = None return if not self.check_major_edge_cases(): + self.feature = await db_client.get_coverage_intersection(self.feature) db_query_result = await db_client.get_building_area(self.feature) raw = db_query_result[0]["area"] or 0 self.area_references["EUBUCCO"] = raw / (1000 * 1000) diff --git a/tests/integrationtests/test_geodatabase.py b/tests/integrationtests/test_geodatabase.py index dd6132440..b0ca21329 100644 --- a/tests/integrationtests/test_geodatabase.py +++ b/tests/integrationtests/test_geodatabase.py @@ -6,7 +6,7 @@ import ohsome_quality_api.geodatabase.client as db_client -pytestmark = pytest.mark.skip("dependency on database setup.") +# pytestmark = pytest.mark.skip("dependency on database setup.") def test_get_connection(): @@ -90,5 +90,12 @@ def test_get_eubucco_coverage_intersection_area(feature_germany_berlin): assert pytest.approx(1.0, 0.1) == result[0]["area_ratio"] +def test_get_coverage_intersection(feature_germany_berlin): + bpoly = feature_germany_berlin + result = asyncio.run(db_client.get_coverage_intersection(bpoly)) + assert result["geometry"].is_valid + assert isinstance(result, geojson.feature.Feature) + + if __name__ == "__main__": unittest.main() From 084c5c787f1734b39d0b5ef3e24433cfc6d4aa43 Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Mon, 13 Nov 2023 13:25:27 +0100 Subject: [PATCH 3/8] test(building-comparison): mock coverage check --- ohsome_quality_api/geodatabase/client.py | 2 +- .../building_comparison/indicator.py | 13 +++-- .../indicators/test_building_comparison.yaml | 57 +++++++++++++++++++ .../indicators/test_building_comparison.py | 26 +++++++++ tests/integrationtests/test_geodatabase.py | 2 +- 5 files changed, 93 insertions(+), 7 deletions(-) diff --git a/ohsome_quality_api/geodatabase/client.py b/ohsome_quality_api/geodatabase/client.py index 7842ba63e..f069e2cea 100644 --- a/ohsome_quality_api/geodatabase/client.py +++ b/ohsome_quality_api/geodatabase/client.py @@ -101,7 +101,7 @@ async def get_eubucco_coverage_intersection_area(bpoly: Feature) -> list[Record] return await conn.fetch(query, geom) -async def get_coverage_intersection(bpoly: Feature) -> Feature: +async def get_eubucco_coverage_intersection(bpoly: Feature) -> Feature: """Get intersection geometry of AoI and coverage geometry.""" file_path = os.path.join(WORKING_DIR, "get_coverage_intersection.sql") with open(file_path, "r") as file: diff --git a/ohsome_quality_api/indicators/building_comparison/indicator.py b/ohsome_quality_api/indicators/building_comparison/indicator.py index 98c458004..cfc8ca378 100644 --- a/ohsome_quality_api/indicators/building_comparison/indicator.py +++ b/ohsome_quality_api/indicators/building_comparison/indicator.py @@ -50,7 +50,9 @@ async def preprocess(self) -> None: self.coverage["EUBUCCO"] = None return if not self.check_major_edge_cases(): - self.feature = await db_client.get_coverage_intersection(self.feature) + self.feature = await db_client.get_eubucco_coverage_intersection( + self.feature + ) db_query_result = await db_client.get_building_area(self.feature) raw = db_query_result[0]["area"] or 0 self.area_references["EUBUCCO"] = raw / (1000 * 1000) @@ -132,7 +134,7 @@ def check_major_edge_cases(self) -> bool: "Reference dataset does not cover area-of-interest." ) return True - elif coverage < 0.50: + elif coverage < 0.10: self.result.description = ( "Only {:.2f}% of the area-of-interest is covered ".format( coverage * 100 @@ -147,10 +149,11 @@ def check_major_edge_cases(self) -> bool: def check_minor_edge_cases(self) -> str: coverage = self.coverage["EUBUCCO"] - if coverage < 0.85: + if coverage < 0.95: return ( - "Warning: Low coverage by the reference dataset (EUBUCCO). " - + "Quality estimation may be inaccurate. " + "Warning: Reference data does not cover the whole input geometry. " + + f"Input geometry is clipped to the coverage. Results is calculated" + f" for {coverage}% of the input geometry." ) else: return "" diff --git a/tests/integrationtests/fixtures/vcr_cassettes/indicators/test_building_comparison.yaml b/tests/integrationtests/fixtures/vcr_cassettes/indicators/test_building_comparison.yaml index 38eec62d1..a7a18e284 100644 --- a/tests/integrationtests/fixtures/vcr_cassettes/indicators/test_building_comparison.yaml +++ b/tests/integrationtests/fixtures/vcr_cassettes/indicators/test_building_comparison.yaml @@ -113,4 +113,61 @@ interactions: - accept-encoding http_version: HTTP/1.1 status_code: 200 +- request: + body: filter=building%3D%2A+and+building%21%3Dno+and+geometry%3Apolygon&bpolys=%7B%22type%22%3A+%22FeatureCollection%22%2C+%22features%22%3A+%5B%7B%22type%22%3A+%22Feature%22%2C+%22geometry%22%3A+%7B%22type%22%3A+%22Polygon%22%2C+%22coordinates%22%3A+%5B%5B%5B13.369514%2C+52.498879%5D%2C+%5B13.373609%2C+52.504164%5D%2C+%5B13.37498%2C+52.503376%5D%2C+%5B13.37765%2C+52.507966%5D%2C+%5B13.39923%2C+52.508077%5D%2C+%5B13.400228%2C+52.509382%5D%2C+%5B13.40803%2C+52.506183%5D%2C+%5B13.409969%2C+52.506928%5D%2C+%5B13.414072%2C+52.504037%5D%2C+%5B13.4272%2C+52.505667%5D%2C+%5B13.429402%2C+52.508563%5D%2C+%5B13.42278%2C+52.512234%5D%2C+%5B13.425927%2C+52.518393%5D%2C+%5B13.429188%2C+52.521203%5D%2C+%5B13.419753%2C+52.525546%5D%2C+%5B13.423642%2C+52.527915%5D%2C+%5B13.438748%2C+52.528778%5D%2C+%5B13.442277%2C+52.531026%5D%2C+%5B13.447168%2C+52.526408%5D%2C+%5B13.452183%2C+52.527796%5D%2C+%5B13.456157%2C+52.522458%5D%2C+%5B13.45529%2C+52.521272%5D%2C+%5B13.462695%2C+52.519928%5D%2C+%5B13.47239%2C+52.520571%5D%2C+%5B13.477729%2C+52.51472%5D%2C+%5B13.475888%2C+52.514863%5D%2C+%5B13.476266%2C+52.510439%5D%2C+%5B13.471164%2C+52.505138%5D%2C+%5B13.468573%2C+52.499657%5D%2C+%5B13.473076%2C+52.499003%5D%2C+%5B13.491443%2C+52.488283%5D%2C+%5B13.482954%2C+52.48605%5D%2C+%5B13.481678%2C+52.487638%5D%2C+%5B13.478628%2C+52.487033%5D%2C+%5B13.476147%2C+52.489944%5D%2C+%5B13.464279%2C+52.493719%5D%2C+%5B13.463355%2C+52.495486%5D%2C+%5B13.45293%2C+52.497707%5D%2C+%5B13.445549%2C+52.494856%5D%2C+%5B13.439262%2C+52.48961%5D%2C+%5B13.4204%2C+52.495864%5D%2C+%5B13.42535%2C+52.488182%5D%2C+%5B13.423705%2C+52.486384%5D%2C+%5B13.407887%2C+52.48886%5D%2C+%5B13.406346%2C+52.482792%5D%2C+%5B13.394613%2C+52.484022%5D%2C+%5B13.394261%2C+52.485775%5D%2C+%5B13.371607%2C+52.484979%5D%2C+%5B13.374019%2C+52.485167%5D%2C+%5B13.373518%2C+52.48797%5D%2C+%5B13.376562%2C+52.49167%5D%2C+%5B13.368229%2C+52.493336%5D%2C+%5B13.369514%2C+52.498879%5D%5D%5D%7D%2C+%22properties%22%3A+%7B%22osm_id%22%3A+-55764%2C+%22boundary%22%3A+%22administrative%22%2C+%22admin_level%22%3A+9%2C+%22parents%22%3A+%22-62422%2C-51477%22%2C+%22name%22%3A+%22Friedrichshain-Kreuzberg%22%2C+%22local_name%22%3A+%22Friedrichshain-Kreuzberg%22%2C+%22name_en%22%3A+null%7D%7D%5D%7D + headers: + accept: + - '*/*' + accept-encoding: + - gzip, deflate + connection: + - keep-alive + content-length: + - '2231' + content-type: + - application/x-www-form-urlencoded + host: + - api.ohsome.org + user-agent: + - ohsome-quality-api/1.0.2 + method: POST + uri: https://api.ohsome.org/v1/elements/area + response: + content: "{\n \"attribution\" : {\n \"url\" : \"https://ohsome.org/copyrights\",\n + \ \"text\" : \"\xA9 OpenStreetMap contributors\"\n },\n \"apiVersion\" : + \"1.10.1\",\n \"result\" : [ {\n \"timestamp\" : \"2023-10-22T20:00:00Z\",\n + \ \"value\" : 5026307.68\n } ]\n}" + headers: + access-control-allow-credentials: + - 'true' + access-control-allow-headers: + - Origin,Accept,X-Requested-With,Content-Type,Access-Control-Request-Method,Access-Control-Request-Headers,Authorization + access-control-allow-methods: + - POST, GET + access-control-allow-origin: + - '*' + access-control-max-age: + - '3600' + cache-control: + - no-cache, no-store, must-revalidate + connection: + - Keep-Alive + content-encoding: + - gzip + content-type: + - application/json + date: + - Mon, 13 Nov 2023 11:14:11 GMT + keep-alive: + - timeout=5, max=100 + server: + - Apache + strict-transport-security: + - max-age=63072000; includeSubdomains; + transfer-encoding: + - chunked + vary: + - accept-encoding + http_version: HTTP/1.1 + status_code: 200 version: 1 diff --git a/tests/integrationtests/indicators/test_building_comparison.py b/tests/integrationtests/indicators/test_building_comparison.py index 1bd9f2ff0..93dcbc900 100644 --- a/tests/integrationtests/indicators/test_building_comparison.py +++ b/tests/integrationtests/indicators/test_building_comparison.py @@ -30,6 +30,24 @@ def mock_get_building_area_empty(class_mocker): ) +@pytest.fixture(scope="class") +def mock_get_eubucco_coverage_intersection_area(class_mocker): + async_mock = AsyncMock(return_value=[{"area_ratio": 1}]) + class_mocker.patch( + "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage_intersection_area", + side_effect=async_mock, + ) + + +@pytest.fixture(scope="class") +def mock_get_eubucco_coverage_intersection(class_mocker, feature_germany_berlin): + async_mock = AsyncMock(return_value=feature_germany_berlin) + class_mocker.patch( + "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage_intersection", + side_effect=async_mock, + ) + + class TestInit: @oqapi_vcr.use_cassette def test_init(self, topic_building_area, feature_germany_berlin): @@ -45,6 +63,8 @@ def test_preprocess( mock_get_building_area, topic_building_area, feature_germany_berlin, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, ): indicator = BuildingComparison(topic_building_area, feature_germany_berlin) asyncio.run(indicator.preprocess()) @@ -65,6 +85,8 @@ def test_calculate( mock_get_building_area, topic_building_area, feature_germany_berlin, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, ): indicator = BuildingComparison(topic_building_area, feature_germany_berlin) asyncio.run(indicator.preprocess()) @@ -80,6 +102,8 @@ def test_calculate_reference_area_0( mock_get_building_area_empty, topic_building_area, feature_germany_heidelberg, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, ): indicator = BuildingComparison(topic_building_area, feature_germany_heidelberg) asyncio.run(indicator.preprocess()) @@ -96,6 +120,8 @@ def indicator( mock_get_building_area, topic_building_area, feature_germany_berlin, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, ): indicator = BuildingComparison(topic_building_area, feature_germany_berlin) asyncio.run(indicator.preprocess()) diff --git a/tests/integrationtests/test_geodatabase.py b/tests/integrationtests/test_geodatabase.py index b0ca21329..50e5c2a40 100644 --- a/tests/integrationtests/test_geodatabase.py +++ b/tests/integrationtests/test_geodatabase.py @@ -92,7 +92,7 @@ def test_get_eubucco_coverage_intersection_area(feature_germany_berlin): def test_get_coverage_intersection(feature_germany_berlin): bpoly = feature_germany_berlin - result = asyncio.run(db_client.get_coverage_intersection(bpoly)) + result = asyncio.run(db_client.get_eubucco_coverage_intersection(bpoly)) assert result["geometry"].is_valid assert isinstance(result, geojson.feature.Feature) From e3feb42cafec983bf97bc8d96225cbf438ad6b3f Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Mon, 13 Nov 2023 13:52:27 +0100 Subject: [PATCH 4/8] test(building-compariosn): fix bug and test --- .../building_comparison/indicator.py | 18 ++++++++++++++---- .../indicators/test_building_comparison.py | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ohsome_quality_api/indicators/building_comparison/indicator.py b/ohsome_quality_api/indicators/building_comparison/indicator.py index cfc8ca378..f3a9ee966 100644 --- a/ohsome_quality_api/indicators/building_comparison/indicator.py +++ b/ohsome_quality_api/indicators/building_comparison/indicator.py @@ -76,11 +76,21 @@ def calculate(self) -> None: else: self.result.description = "" - self.result.value = float( - mean([self.area_osm / v for v in self.area_references.values()]) - ) + if all(v == 0 for v in self.area_references.values()): + pass + else: + self.result.value = float( + mean( + [self.area_osm / v for v in self.area_references.values() if v != 0] + ) + ) - if self.result.value >= self.th_high: + if self.result.value is None: + self.result.description += ( + "\n" + self.metadata.label_description[self.result.label] + ) + return + elif self.result.value >= self.th_high: self.result.class_ = 5 elif self.result.value >= self.th_low: self.result.class_ = 3 diff --git a/tests/integrationtests/indicators/test_building_comparison.py b/tests/integrationtests/indicators/test_building_comparison.py index 93dcbc900..360abfc8c 100644 --- a/tests/integrationtests/indicators/test_building_comparison.py +++ b/tests/integrationtests/indicators/test_building_comparison.py @@ -23,7 +23,7 @@ def mock_get_building_area(class_mocker): @pytest.fixture(scope="class") def mock_get_building_area_empty(class_mocker): - async_mock = AsyncMock(return_value=[]) + async_mock = AsyncMock(return_value=[{"area": 0}]) class_mocker.patch( "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_building_area", side_effect=async_mock, From f8bac259a07c68a2c42ff4a4c19da8001014cfcf Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Mon, 13 Nov 2023 14:37:30 +0100 Subject: [PATCH 5/8] tests(building-comparison): fix thresholds and add above_one_th --- .../building_comparison/indicator.py | 12 +++- .../indicators/test_building_comparison.py | 66 +++++++++++++++++++ 2 files changed, 75 insertions(+), 3 deletions(-) diff --git a/ohsome_quality_api/indicators/building_comparison/indicator.py b/ohsome_quality_api/indicators/building_comparison/indicator.py index f3a9ee966..e27186245 100644 --- a/ohsome_quality_api/indicators/building_comparison/indicator.py +++ b/ohsome_quality_api/indicators/building_comparison/indicator.py @@ -32,6 +32,7 @@ def __init__( # TODO: Evaluate thresholds self.th_high = 0.85 # Above or equal to this value label should be green self.th_low = 0.50 # Above or equal to this value label should be yellow + self.above_one_th = 1.30 @classmethod async def coverage(cls) -> Polygon | MultiPolygon: @@ -90,12 +91,17 @@ def calculate(self) -> None: "\n" + self.metadata.label_description[self.result.label] ) return - elif self.result.value >= self.th_high: + elif self.above_one_th >= self.result.value >= self.th_high: self.result.class_ = 5 - elif self.result.value >= self.th_low: + elif self.th_high > self.result.value >= self.th_low: self.result.class_ = 3 - else: + elif self.th_low > self.result.value >= 0: self.result.class_ = 1 + elif self.result.value > self.above_one_th: + self.result.description += ( + "Warning: No quality estimation made. " + "OSM and reference data differ. Reference data is likely outdated." + ) template = Template(self.metadata.result_description) self.result.description += template.substitute( diff --git a/tests/integrationtests/indicators/test_building_comparison.py b/tests/integrationtests/indicators/test_building_comparison.py index 360abfc8c..82f5479b2 100644 --- a/tests/integrationtests/indicators/test_building_comparison.py +++ b/tests/integrationtests/indicators/test_building_comparison.py @@ -21,6 +21,15 @@ def mock_get_building_area(class_mocker): ) +@pytest.fixture(scope="class") +def mock_get_building_area_low(class_mocker): + async_mock = AsyncMock(return_value=[{"area": 1}]) + class_mocker.patch( + "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_building_area", + side_effect=async_mock, + ) + + @pytest.fixture(scope="class") def mock_get_building_area_empty(class_mocker): async_mock = AsyncMock(return_value=[{"area": 0}]) @@ -111,6 +120,29 @@ def test_calculate_reference_area_0( indicator.calculate() assert indicator.result.value is None + @oqapi_vcr.use_cassette + def test_calculate_above_one_th( + self, + mock_get_building_area_low, + topic_building_area, + feature_germany_heidelberg, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, + ): + indicator = BuildingComparison(topic_building_area, feature_germany_heidelberg) + asyncio.run(indicator.preprocess()) + indicator.calculate() + assert indicator.result.value is not None + assert indicator.result.value > 0 + assert indicator.result.class_ is None + assert indicator.result.description is not None + assert ( + "Warning: No quality estimation made. " + "OSM and reference data differ. Reference data is likely outdated." + in indicator.result.description + ) + assert indicator.result.label == "undefined" + class TestFigure: @pytest.fixture(scope="class") @@ -137,3 +169,37 @@ def test_create_figure(self, indicator): def test_create_figure_manual(self, indicator): indicator.create_figure() pio.show(indicator.result.figure) + + @oqapi_vcr.use_cassette + def test_create_figure_above_one_th( + self, + mock_get_building_area_low, + topic_building_area, + feature_germany_berlin, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, + ): + indicator = BuildingComparison(topic_building_area, feature_germany_berlin) + asyncio.run(indicator.preprocess()) + indicator.calculate() + indicator.create_figure() + assert isinstance(indicator.result.figure, dict) + assert indicator.result.figure["data"][0]["type"] == "bar" + pgo.Figure(indicator.result.figure) + + @oqapi_vcr.use_cassette + def test_create_figure_building_area_zero( + self, + mock_get_building_area_empty, + topic_building_area, + feature_germany_berlin, + mock_get_eubucco_coverage_intersection_area, + mock_get_eubucco_coverage_intersection, + ): + indicator = BuildingComparison(topic_building_area, feature_germany_berlin) + asyncio.run(indicator.preprocess()) + indicator.calculate() + indicator.create_figure() + assert isinstance(indicator.result.figure, dict) + assert indicator.result.figure["data"][0]["type"] == "bar" + pgo.Figure(indicator.result.figure) From bebd321df3f5987b7b3554d72f630fdd472c6c9a Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Mon, 13 Nov 2023 14:47:10 +0100 Subject: [PATCH 6/8] chore: add changelog --- CHANGELOG.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a1ad6402..fb39deadf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## Current Main + +### Bug Fixes + +- building-comparison: is calculated for coverage area only ([#739]) +- building-comparison: result description now shows correct coverage percentage ([#739]) + +### New Features + +- building-comparison: AOI is now clipped to the coverage area ([#739]) + +### Other Changes + +- building-comparison: no quality estimation for areas with strong difference to reference data ([#739]) + + ## Release 1.0.2 ### Bug Fixes From 62b9ad865eb40cef508575733ba6f229dedb11c6 Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Mon, 13 Nov 2023 17:00:15 +0100 Subject: [PATCH 7/8] tests: mock tests with database requests --- .../indicators/building_comparison/indicator.py | 6 ++---- .../indicators/test_building_comparison.py | 16 ++++++++-------- tests/integrationtests/test_geodatabase.py | 2 +- tests/unittests/test_indicators_definitions.py | 15 ++++++++++++++- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/ohsome_quality_api/indicators/building_comparison/indicator.py b/ohsome_quality_api/indicators/building_comparison/indicator.py index e27186245..c1efee7df 100644 --- a/ohsome_quality_api/indicators/building_comparison/indicator.py +++ b/ohsome_quality_api/indicators/building_comparison/indicator.py @@ -78,6 +78,7 @@ def calculate(self) -> None: self.result.description = "" if all(v == 0 for v in self.area_references.values()): + self.result.description += "Warning: No reference data in this area. " pass else: self.result.value = float( @@ -87,9 +88,6 @@ def calculate(self) -> None: ) if self.result.value is None: - self.result.description += ( - "\n" + self.metadata.label_description[self.result.label] - ) return elif self.above_one_th >= self.result.value >= self.th_high: self.result.class_ = 5 @@ -100,7 +98,7 @@ def calculate(self) -> None: elif self.result.value > self.above_one_th: self.result.description += ( "Warning: No quality estimation made. " - "OSM and reference data differ. Reference data is likely outdated." + "OSM and reference data differ. Reference data is likely outdated. " ) template = Template(self.metadata.result_description) diff --git a/tests/integrationtests/indicators/test_building_comparison.py b/tests/integrationtests/indicators/test_building_comparison.py index 82f5479b2..70f60e370 100644 --- a/tests/integrationtests/indicators/test_building_comparison.py +++ b/tests/integrationtests/indicators/test_building_comparison.py @@ -14,7 +14,7 @@ @pytest.fixture(scope="class") def mock_get_building_area(class_mocker): - async_mock = AsyncMock(return_value=[{"area": 4842587.791645115 / (1000 * 1000)}]) + async_mock = AsyncMock(return_value=[{"area": 4842587.791645115}]) class_mocker.patch( "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_building_area", side_effect=async_mock, @@ -40,19 +40,19 @@ def mock_get_building_area_empty(class_mocker): @pytest.fixture(scope="class") -def mock_get_eubucco_coverage_intersection_area(class_mocker): - async_mock = AsyncMock(return_value=[{"area_ratio": 1}]) +def mock_get_eubucco_coverage_intersection(class_mocker, feature_germany_berlin): + async_mock = AsyncMock(return_value=feature_germany_berlin) class_mocker.patch( - "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage_intersection_area", + "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage_intersection", side_effect=async_mock, ) @pytest.fixture(scope="class") -def mock_get_eubucco_coverage_intersection(class_mocker, feature_germany_berlin): - async_mock = AsyncMock(return_value=feature_germany_berlin) +def mock_get_eubucco_coverage_intersection_area(class_mocker): + async_mock = AsyncMock(return_value=[{"area_ratio": 1}]) class_mocker.patch( - "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage_intersection", + "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage_intersection_area", side_effect=async_mock, ) @@ -81,7 +81,7 @@ def test_preprocess( assert indicator.area_osm is not None assert indicator.area_osm > 0 assert indicator.area_references == { - "EUBUCCO": 4.842587791645116 / (1000 * 1000) + "EUBUCCO": 4.842587791645116, } assert isinstance(indicator.result.timestamp, datetime) assert isinstance(indicator.result.timestamp_osm, datetime) diff --git a/tests/integrationtests/test_geodatabase.py b/tests/integrationtests/test_geodatabase.py index 50e5c2a40..8b27feddf 100644 --- a/tests/integrationtests/test_geodatabase.py +++ b/tests/integrationtests/test_geodatabase.py @@ -6,7 +6,7 @@ import ohsome_quality_api.geodatabase.client as db_client -# pytestmark = pytest.mark.skip("dependency on database setup.") +pytestmark = pytest.mark.skip("dependency on database setup.") def test_get_connection(): diff --git a/tests/unittests/test_indicators_definitions.py b/tests/unittests/test_indicators_definitions.py index b7ff76a66..2989b6588 100644 --- a/tests/unittests/test_indicators_definitions.py +++ b/tests/unittests/test_indicators_definitions.py @@ -1,10 +1,23 @@ import asyncio +from unittest.mock import AsyncMock import geojson +import pytest from ohsome_quality_api.indicators import definitions, models +@pytest.fixture(scope="class") +def mock_select_eubucco_coverage(class_mocker, feature_germany_berlin): + async_mock = AsyncMock( + return_value=[{"geom": geojson.dumps(feature_germany_berlin)}] + ) + class_mocker.patch( + "ohsome_quality_api.indicators.building_completeness.indicator.db_client.get_eubucco_coverage", + side_effect=async_mock, + ) + + def test_get_indicator_names(): names = definitions.get_indicator_keys() assert isinstance(names, list) @@ -30,7 +43,7 @@ def test_get_indicator_definitions_with_project(): assert indicator.projects == ["core"] -def test_get_coverage(): +def test_get_coverage(mock_select_eubucco_coverage): coverage = asyncio.run(definitions.get_coverage("building-comparison")) assert coverage.is_valid assert isinstance(coverage, geojson.FeatureCollection) From e5941bf3d5ad848583cbfc0bbc73841cb13da16b Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 14 Nov 2023 14:16:39 +0100 Subject: [PATCH 8/8] chore: update changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb39deadf..6d4ddbfad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ ### Other Changes - building-comparison: no quality estimation for areas with strong difference to reference data ([#739]) +- test(db): add missing mock for getting coverage from database in tests ([#739]) + +[#739]: https://github.com/GIScience/ohsome-quality-api/pull/739 ## Release 1.0.2