From a851ae991173463479fc5a0fa2e5b7134ad61ca2 Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 12 Jul 2022 17:19:49 +0200 Subject: [PATCH 01/14] wip --- workers/ohsome_quality_analyst/api/api.py | 9 -- .../api/request_models.py | 47 +++++++---- .../ohsome_quality_analyst/base/indicator.py | 23 +++++- workers/ohsome_quality_analyst/cli/cli.py | 13 +-- workers/ohsome_quality_analyst/definitions.py | 82 ++++--------------- .../building_completeness/indicator.py | 5 ++ .../building_completeness/metadata.yaml | 2 + .../indicators/currentness/metadata.yaml | 12 ++- .../metadata.yaml | 4 +- .../ghs_pop_comparison_roads/metadata.yaml | 2 + .../mapping_saturation/metadata.yaml | 7 ++ .../indicators/minimal/indicator.py | 4 +- .../indicators/minimal/metadata.yaml | 8 ++ .../indicators/poi_density/metadata.yaml | 2 + .../indicators/tags_ratio/metadata.yaml | 3 + workers/ohsome_quality_analyst/oqt.py | 5 +- .../reports/minimal/metadata.yaml | 2 +- 17 files changed, 115 insertions(+), 115 deletions(-) diff --git a/workers/ohsome_quality_analyst/api/api.py b/workers/ohsome_quality_analyst/api/api.py index 1757e2fe6..7c69a4b84 100644 --- a/workers/ohsome_quality_analyst/api/api.py +++ b/workers/ohsome_quality_analyst/api/api.py @@ -32,7 +32,6 @@ from ohsome_quality_analyst.config import configure_logging from ohsome_quality_analyst.definitions import ( ATTRIBUTION_URL, - INDICATOR_LAYER, get_attribution, get_dataset_names, get_fid_fields, @@ -280,14 +279,6 @@ async def get_available_regions(asGeoJSON: bool = False): return response -@app.get("/indicator-layer-combinations") -async def get_indicator_layer_combinations(): - """Get names of available indicator-layer combinations.""" - response = empty_api_response() - response["result"] = INDICATOR_LAYER - return response - - @app.get("/indicators") async def indicator_names(): """Get names of available indicators.""" diff --git a/workers/ohsome_quality_analyst/api/request_models.py b/workers/ohsome_quality_analyst/api/request_models.py index 527af57d3..d3f521d77 100644 --- a/workers/ohsome_quality_analyst/api/request_models.py +++ b/workers/ohsome_quality_analyst/api/request_models.py @@ -8,7 +8,7 @@ """ from enum import Enum -from typing import Optional, Union +from typing import Optional, Tuple, Union import pydantic from geojson import Feature, FeatureCollection @@ -16,12 +16,12 @@ from ohsome_quality_analyst.base.layer import LayerData from ohsome_quality_analyst.definitions import ( - INDICATOR_LAYER, get_dataset_names, get_fid_fields, get_indicator_names, get_layer_keys, get_report_names, + get_valid_layers, ) from ohsome_quality_analyst.utils.helper import loads_geojson, snake_to_lower_camel @@ -36,6 +36,14 @@ class BaseIndicator(BaseModel): name: IndicatorEnum = pydantic.Field( ..., title="Indicator Name", example="GhsPopComparisonBuildings" ) + threshholds: Optional[ + Tuple[ + Union[float, str], + Union[str, float], + Union[str, float], + Union[str, float], + ] + ] = None include_svg: bool = False include_html: bool = False include_data: bool = False @@ -48,6 +56,17 @@ class Config: allow_mutation = False extra = "forbid" + @pydantic.root_validator + @classmethod + def validate_thresholds(cls, values): + if values["threshholds"] is not None and values["name"] != "Currentness": + raise ValueError( + "Setting custom threshholds is only supported for the Currentness " + + "Indicator.", + ) + else: + return values + class BaseReport(BaseModel): name: ReportEnum = pydantic.Field( @@ -131,13 +150,13 @@ class IndicatorBpolys(BaseIndicator, BaseLayerName, BaseBpolys): @pydantic.root_validator @classmethod def validate_indicator_layer(cls, values): - try: - indicator_layer = (values["name"].value, values["layer_key"].value) - except KeyError: - raise ValueError("An issue with the layer or indicator name occurred.") - if indicator_layer not in INDICATOR_LAYER: + indicator_key = values["name"].value + layer_key = values["layer_key"].value + if layer_key not in get_valid_layers(indicator_key): raise ValueError( - "Indicator layer combination is invalid: " + str(indicator_layer) + "Layer ({0}) is not available for indicator ({1})".format( + layer_key, indicator_key + ) ) else: return values @@ -147,13 +166,13 @@ class IndicatorDatabase(BaseIndicator, BaseLayerName, BaseDatabase): @pydantic.root_validator @classmethod def validate_indicator_layer(cls, values): - try: - indicator_layer = (values["name"].value, values["layer_key"].value) - except KeyError: - raise ValueError("An issue with the layer or indicator name occurred.") - if indicator_layer not in INDICATOR_LAYER: + indicator_key = values["name"].value + layer_key = values["layer_key"].value + if layer_key not in get_valid_layers(indicator_key): raise ValueError( - "Indicator layer combination is invalid: " + str(indicator_layer) + "Layer ({0}) is not available for indicator ({1})".format( + layer_key, indicator_key + ) ) else: return values diff --git a/workers/ohsome_quality_analyst/base/indicator.py b/workers/ohsome_quality_analyst/base/indicator.py index 3bfdcf12c..8ee05fb47 100644 --- a/workers/ohsome_quality_analyst/base/indicator.py +++ b/workers/ohsome_quality_analyst/base/indicator.py @@ -5,7 +5,7 @@ from dataclasses import asdict, dataclass from datetime import datetime, timezone from io import StringIO -from typing import Dict, Literal, Optional +from typing import Dict, Literal, Optional, Tuple import matplotlib.pyplot as plt from dacite import from_dict @@ -42,8 +42,8 @@ class Result: value is determined by the result classes value (float): The result value class_ (int): The result class. An integer between 1 and 5. It maps to the - result labels. This value is used by the reports to determine an overall - result. + result labels (1 -> red; 5 -> green). This value is used by the reports to + determine an overall result. description (str): The result description. svg (str): Figure of the result as SVG """ @@ -63,17 +63,32 @@ def label(self) -> Literal["green", "yellow", "red", "undefined"]: class BaseIndicator(metaclass=ABCMeta): - """The base class of every indicator.""" + """The base class of every indicator. + + Attributes: + thresholds (tuple): A tuple with four float values representing the thresholds + between the result classes. The first element is the threshold between the + result class 1 and 2, the second element is the threshold between the result + class 2 and 3 and so on. + """ def __init__( self, layer: Layer, feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: self.layer: Layer = layer self.feature: Feature = feature + # setattr(object, key, value) could be used instead of relying on from_dict. metadata = get_metadata("indicators", type(self).__name__) + layer_thresholds = metadata.pop("layer-thresholds") + if thresholds is None: + # TODO: filter layer_thresholds + self.thresholds = layer_thresholds[layer.key] + else: + self.thresholds = thresholds self.metadata: Metadata = from_dict(data_class=Metadata, data=metadata) self.result: Result = Result( description=self.metadata.label_description["undefined"], diff --git a/workers/ohsome_quality_analyst/cli/cli.py b/workers/ohsome_quality_analyst/cli/cli.py index e5e05fc0f..13543a4ae 100644 --- a/workers/ohsome_quality_analyst/cli/cli.py +++ b/workers/ohsome_quality_analyst/cli/cli.py @@ -15,11 +15,7 @@ ) from ohsome_quality_analyst.cli import options from ohsome_quality_analyst.config import configure_logging, get_config_value -from ohsome_quality_analyst.definitions import ( - INDICATOR_LAYER, - load_layer_definitions, - load_metadata, -) +from ohsome_quality_analyst.definitions import load_layer_definitions, load_metadata from ohsome_quality_analyst.geodatabase import client as db_client from ohsome_quality_analyst.utils.helper import json_serialize, write_geojson @@ -93,13 +89,6 @@ def get_available_regions(): click.echo(format_row.format(region["ogc_fid"], region["name"])) -@cli.command("list-indicator-layer-combination") -def get_indicator_layer_combination(): - """List all possible indicator-layer-combinations.""" - for combination in INDICATOR_LAYER: - click.echo(combination) - - @cli.command("create-indicator") @cli_option(options.indicator_name) @cli_option(options.layer_key) diff --git a/workers/ohsome_quality_analyst/definitions.py b/workers/ohsome_quality_analyst/definitions.py index 49e667a48..54b2db184 100644 --- a/workers/ohsome_quality_analyst/definitions.py +++ b/workers/ohsome_quality_analyst/definitions.py @@ -1,10 +1,12 @@ """Global Variables and Functions.""" +from __future__ import annotations + import glob import logging import os from dataclasses import dataclass from types import MappingProxyType -from typing import Dict, List, Optional +from typing import Dict, List, Literal, Optional, Tuple import yaml @@ -57,63 +59,6 @@ class RasterDataset: ), ) -# Possible indicator layer combinations -INDICATOR_LAYER = ( - ("BuildingCompleteness", "building_area"), - ("GhsPopComparisonBuildings", "building_count"), - ("GhsPopComparisonRoads", "jrc_road_length"), - ("GhsPopComparisonRoads", "major_roads_length"), - ("MappingSaturation", "building_count"), - ("MappingSaturation", "major_roads_length"), - ("MappingSaturation", "amenities"), - ("MappingSaturation", "jrc_health_count"), - ("MappingSaturation", "jrc_mass_gathering_sites_count"), - ("MappingSaturation", "jrc_railway_length"), - ("MappingSaturation", "jrc_road_length"), - ("MappingSaturation", "jrc_education_count"), - ("MappingSaturation", "mapaction_settlements_count"), - ("MappingSaturation", "mapaction_major_roads_length"), - ("MappingSaturation", "mapaction_rail_length"), - ("MappingSaturation", "mapaction_lakes_area"), - ("MappingSaturation", "mapaction_rivers_length"), - ("MappingSaturation", "infrastructure_lines"), - ("MappingSaturation", "poi"), - ("MappingSaturation", "lulc"), - ("Currentness", "major_roads_count"), - ("Currentness", "building_count"), - ("Currentness", "amenities"), - ("Currentness", "jrc_health_count"), - ("Currentness", "jrc_education_count"), - ("Currentness", "jrc_road_count"), - ("Currentness", "jrc_railway_count"), - ("Currentness", "jrc_airport_count"), - ("Currentness", "jrc_water_treatment_plant_count"), - ("Currentness", "jrc_power_generation_plant_count"), - ("Currentness", "jrc_cultural_heritage_site_count"), - ("Currentness", "jrc_bridge_count"), - ("Currentness", "jrc_mass_gathering_sites_count"), - ("Currentness", "mapaction_settlements_count"), - ("Currentness", "mapaction_major_roads_length"), - ("Currentness", "mapaction_rail_length"), - ("Currentness", "mapaction_lakes_count"), - ("Currentness", "mapaction_rivers_length"), - ("Currentness", "infrastructure_lines"), - ("Currentness", "poi"), - ("Currentness", "lulc"), - ("PoiDensity", "poi"), - ("TagsRatio", "building_count"), - ("TagsRatio", "major_roads_length"), - ("TagsRatio", "jrc_health_count"), - ("TagsRatio", "jrc_education_count"), - ("TagsRatio", "jrc_road_length"), - ("TagsRatio", "jrc_airport_count"), - ("TagsRatio", "jrc_power_generation_plant_count"), - ("TagsRatio", "jrc_cultural_heritage_site_count"), - ("TagsRatio", "jrc_bridge_count"), - ("TagsRatio", "jrc_mass_gathering_sites_count"), - ("Minimal", "minimal"), -) - ATTRIBUTION_TEXTS = MappingProxyType( { "OSM": "© OpenStreetMap contributors", @@ -128,17 +73,16 @@ class RasterDataset: ) -def load_metadata(module_name: str) -> Dict: - """Read metadata of all indicators or reports from YAML files. +def load_metadata(module_name: Literal["indicators", "reports"]) -> Dict: + """Load metadata of all indicators or reports from YAML files. - Those text files are located in the directory of each indicator/report. + The YAML files are located in the directory of each individual indicator or report. - Args: - module_name: Either indicators or reports. Returns: - A Dict with the class names of the indicators/reports - as keys and metadata as values. + A dictionary with the indicator or report keys as directory keys and the content + of the YAML file (metadata) as values. """ + # TODO: Is this check needed if Literal is used in func declaration? if module_name != "indicators" and module_name != "reports": raise ValueError("module name value can only be 'indicators' or 'reports'.") @@ -275,11 +219,15 @@ def get_attribution(data_keys: list) -> str: return "; ".join([str(v) for v in filtered.values()]) +# TODO def get_valid_layers(indcator_name: str) -> tuple: """Get valid Indicator/Layer combination of an Indicator.""" - return tuple([tup[1] for tup in INDICATOR_LAYER if tup[0] == indcator_name]) + return tuple( + [tup[1] for tup in INDICATOR_LAYER_THRESHOLDS if tup[0] == indcator_name] + ) +# TODO def get_valid_indicators(layer_key: str) -> tuple: """Get valid Indicator/Layer combination of a Layer.""" - return tuple([tup[0] for tup in INDICATOR_LAYER if tup[1] == layer_key]) + return tuple([tup[0] for tup in INDICATOR_LAYER_THRESHOLDS if tup[1] == layer_key]) diff --git a/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py b/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py index 9177e63e0..5adc29153 100644 --- a/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py @@ -2,6 +2,7 @@ import os from io import StringIO from string import Template +from typing import Optional, Tuple import dateutil.parser import geojson @@ -58,10 +59,14 @@ def __init__( self, layer: Layer, feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]], ) -> None: + if thresholds is None: + thresholds = (0.2, 0.5, 0.8, 0.9) super().__init__( layer=layer, feature=feature, + thresholds=thresholds, ) self.model_name: str = "Random Forest Regressor" # Lists of elements per hexagonal cell diff --git a/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml b/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml index 9710d8e1e..2856c8843 100644 --- a/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml @@ -23,3 +23,5 @@ BuildingCompleteness: average of the ratios per hex-cell between the building area mapped in OSM and the predicted building area is $completeness_ratio %. The weight is the predicted building area. + layer-thresholds: + - { layer: building_area, thresholds: [0.2, 0.5, 0.8, 0.9] } diff --git a/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml b/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml index 3b827991e..e03ac7ce0 100644 --- a/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml @@ -19,7 +19,11 @@ Currentness: an extrinsic comparison to identify if this means that data quality is bad or if there is just nothing to map here. result_description: | - In the last $threshold_green years $green % of the elements were edited the last time. - In the period from $threshold_yellow_start to $threshold_yellow_end years ago $yellow % of the elements were edited the last time. - The remaining $red % were last edited more than $threshold_red years ago. - The median currentness of the $elements features ($layer_name) is $median_years year(s). + Over 50% of the $elements features ($layer_name) were edited $years. + layer-thresholds: + - { layer: amenities, thresholds: [ 0.2, null, 0.6, null] } + - { layer: building_count, thresholds: [ 0.2, null, 0.6, null] } + - { layer: major_roads_count, thresholds: [ 0.2, null, 0.6, null] } + - { layer: infrastructure_lines, thresholds: [ 0.2, null, 0.6, null] } + - { layer: poi, thresholds: [ 0.2, null, 0.6, null] } + - { layer: lulc, thresholds: [ 0.2, null, 0.6, null] } diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml index 36d689f8c..b655ea660 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml @@ -23,4 +23,6 @@ GhsPopComparisonBuildings: $pop_count people living in an area of $area sqkm, which results in a population density $pop_count_per_sqkm of people per sqkm. - $feature_count_per_sqkm buildings per sqkm mapped. + $feature_count_per_sqkm buildings per sqkm mapped. + layer-thresholds: + - { layer: building_count, thresholds: [{ a:0.75 }, null, { a: 5.0 }, null] } diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml index d1e7a4c5c..aec0b4f12 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml @@ -24,3 +24,5 @@ GhsPopComparisonRoads: $area sqkm, which results in a population density $pop_count_per_sqkm of people per sqkm. $feature_length_per_sqkm km of roads per sqkm mapped. + layer-thresholds: + - { layer: major_roads_length, thresholds: [{ a: 1000 }, null, { a: 500 }, null] } diff --git a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml index 1e1dfe09d..6182d9fa0 100644 --- a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml @@ -15,3 +15,10 @@ MappingSaturation: Saturation could not be calculated. result_description: | The saturation of the last 3 years is $saturation%. + layer-thresholds: + - { layer: amenities, thresholds: [0.3, null, 0.97, null] } + - { layer: building_count, thresholds: [0.3, null, 0.97, null] } + - { layer: infrastructure_lines, thresholds: [0.3, null, 0.97, null] } + - { layer: lulc, thresholds: [0.3, null, 0.97, null] } + - { layer: major_roads_length, thresholds: [0.3, null, 0.97, null] } + - { layer: poi, thresholds: [0.3, null, 0.97, null] } diff --git a/workers/ohsome_quality_analyst/indicators/minimal/indicator.py b/workers/ohsome_quality_analyst/indicators/minimal/indicator.py index 118e7617f..9c9835bb0 100644 --- a/workers/ohsome_quality_analyst/indicators/minimal/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/minimal/indicator.py @@ -10,8 +10,8 @@ class Minimal(BaseIndicator): - def __init__(self, layer: Layer, feature: Feature) -> None: - super().__init__(layer=layer, feature=feature) + def __init__(self, layer: Layer, feature: Feature, thresholds: tuple) -> None: + super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.count = 0 async def preprocess(self) -> None: diff --git a/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml b/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml index 0dfc4bd8e..3037c6014 100644 --- a/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml @@ -14,3 +14,11 @@ Minimal: The quality level could not be calculated for this indicator. result_description: | Some description of the result. + layer-thresholds: + # List of associative lists of valid layers and thresholds. + # A threshold is a list of four float values. + # The first element is the threshold between the result class 1 + # (maps to the label 'red'). + - { layer: minimal, thresholds: [0.2, 0.4, 0.6, 0.8] } + # Thresholds can also be functins parameters + # thresholds: [ {a: 0.2, b: 0.4,} { a: 0.6, b: 0.8 } ... ] diff --git a/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml b/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml index 86863da0d..c9da5bdba 100644 --- a/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml @@ -22,3 +22,5 @@ PoiDensity: The density of landmarks (points of reference, e.g. waterbodies, supermarkets, churches, bus stops) is $result features per sqkm. + layer-thresholds: + - { layer: poi, thresholds: [10, null, null, 30] } diff --git a/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml b/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml index 6e7f27ac6..7a9f7f5b0 100644 --- a/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml @@ -17,3 +17,6 @@ TagsRatio: result_description: | The ratio of the features (all: $all) compared to features with expected tags (matched: $matched) is $result. + layer-thresholds: + - { layer: building_count, thresholds: [ 0.25, null, 0.75, null] } + - { layer: major_roads_length, thresholds: [ 0.25, null, 0.75, null] } diff --git a/workers/ohsome_quality_analyst/oqt.py b/workers/ohsome_quality_analyst/oqt.py index b257a4f29..f6f6a33ba 100644 --- a/workers/ohsome_quality_analyst/oqt.py +++ b/workers/ohsome_quality_analyst/oqt.py @@ -183,6 +183,7 @@ async def _( IndicatorBpolys( name=name, layerKey=parameters.layer_key.value, + threshholds=parameters.threshholds, bpolys=feature, ) ) @@ -200,6 +201,7 @@ async def _( name = parameters.name.value layer: Layer = get_layer_definition(parameters.layer_key.value) feature = parameters.bpolys + threshholds = parameters.threshholds logging.info("Calculating Indicator for custom AOI ...") logging.info("Feature id: {0:4}".format(feature.get("id", 1))) @@ -207,7 +209,7 @@ async def _( logging.info("Layer name: {0:4}".format(layer.name)) indicator_class = name_to_class(class_type="indicator", name=name) - indicator = indicator_class(layer, feature) + indicator = indicator_class(layer, feature, threshholds) logging.info("Run preprocessing") await indicator.preprocess() @@ -364,6 +366,7 @@ async def create_all_indicators( elif indicator_name is not None and layer_key is not None: indicator_layer = [(indicator_name, layer_key)] else: + # TODO indicator_layer = INDICATOR_LAYER tasks: List[asyncio.Task] = [] diff --git a/workers/ohsome_quality_analyst/reports/minimal/metadata.yaml b/workers/ohsome_quality_analyst/reports/minimal/metadata.yaml index 5c32447d7..5977fa20d 100644 --- a/workers/ohsome_quality_analyst/reports/minimal/metadata.yaml +++ b/workers/ohsome_quality_analyst/reports/minimal/metadata.yaml @@ -2,7 +2,7 @@ Minimal: name: Minimal description: | - This report shows the quality for two indicators: + This report shows the quality for two indicators":" Mapping Saturation and Currentness. It's main function is to test the interactions between database, api and website. From f681323e38a90747686be1c4e728fa755022a008 Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Tue, 20 Sep 2022 15:46:02 +0200 Subject: [PATCH 02/14] chore: merge main --- workers/ohsome_quality_analyst/definitions.py | 16 ++++++++-------- workers/ohsome_quality_analyst/oqt.py | 3 +-- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/workers/ohsome_quality_analyst/definitions.py b/workers/ohsome_quality_analyst/definitions.py index 54b2db184..1ea68cb5b 100644 --- a/workers/ohsome_quality_analyst/definitions.py +++ b/workers/ohsome_quality_analyst/definitions.py @@ -6,7 +6,7 @@ import os from dataclasses import dataclass from types import MappingProxyType -from typing import Dict, List, Literal, Optional, Tuple +from typing import Dict, List, Literal, Optional import yaml @@ -220,14 +220,14 @@ def get_attribution(data_keys: list) -> str: # TODO -def get_valid_layers(indcator_name: str) -> tuple: - """Get valid Indicator/Layer combination of an Indicator.""" - return tuple( - [tup[1] for tup in INDICATOR_LAYER_THRESHOLDS if tup[0] == indcator_name] - ) +def get_valid_layers(indicator_name: str) -> tuple: + """Get valid Indicator/Layer combination of an indicator.""" + indicator_layer = load_metadata("indicators") + return tuple([tup[1] for tup in indicator_layer if tup[0] == indicator_name]) # TODO def get_valid_indicators(layer_key: str) -> tuple: - """Get valid Indicator/Layer combination of a Layer.""" - return tuple([tup[0] for tup in INDICATOR_LAYER_THRESHOLDS if tup[1] == layer_key]) + """Get valid Indicator/Layer combination of a layer.""" + indicator_layer = load_metadata("indicators") + return tuple([tup[0] for tup in indicator_layer if tup[1] == layer_key]) diff --git a/workers/ohsome_quality_analyst/oqt.py b/workers/ohsome_quality_analyst/oqt.py index f6f6a33ba..9e01a2d06 100644 --- a/workers/ohsome_quality_analyst/oqt.py +++ b/workers/ohsome_quality_analyst/oqt.py @@ -23,7 +23,6 @@ from ohsome_quality_analyst.base.report import BaseReport as Report from ohsome_quality_analyst.config import get_config_value from ohsome_quality_analyst.definitions import ( - INDICATOR_LAYER, get_layer_definition, get_valid_indicators, get_valid_layers, @@ -367,7 +366,7 @@ async def create_all_indicators( indicator_layer = [(indicator_name, layer_key)] else: # TODO - indicator_layer = INDICATOR_LAYER + indicator_layer = "INDICATOR_LAYER" tasks: List[asyncio.Task] = [] fids = await db_client.get_feature_ids(dataset) From ba8d153ec4c3417bcb5ba25ec6c4e8bf14361caf Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Wed, 21 Sep 2022 16:56:41 +0200 Subject: [PATCH 03/14] fix: get_valid_indicators and get_valid_layers --- workers/ohsome_quality_analyst/definitions.py | 15 ++++++++-- .../indicators/minimal/indicator.py | 4 ++- .../indicators/minimal/metadata.yaml | 2 +- workers/tests/integrationtests/test_oqt.py | 28 +++++++++++++++++++ 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/workers/ohsome_quality_analyst/definitions.py b/workers/ohsome_quality_analyst/definitions.py index 1ea68cb5b..85fb4fd3e 100644 --- a/workers/ohsome_quality_analyst/definitions.py +++ b/workers/ohsome_quality_analyst/definitions.py @@ -220,14 +220,23 @@ def get_attribution(data_keys: list) -> str: # TODO -def get_valid_layers(indicator_name: str) -> tuple: +def get_valid_layers(indicator_name: str) -> list: """Get valid Indicator/Layer combination of an indicator.""" indicator_layer = load_metadata("indicators") - return tuple([tup[1] for tup in indicator_layer if tup[0] == indicator_name]) + valid_layers = [] + for key, value in indicator_layer.items(): + if key == indicator_name: + for comb in value["layer-thresholds"]: + valid_layers.append(comb["layer"]) + return valid_layers # TODO def get_valid_indicators(layer_key: str) -> tuple: """Get valid Indicator/Layer combination of a layer.""" indicator_layer = load_metadata("indicators") - return tuple([tup[0] for tup in indicator_layer if tup[1] == layer_key]) + valid_indicators = [] + for key, value in indicator_layer.items(): + if any(comb["layer"] == layer_key for comb in value["layer-thresholds"]): + valid_indicators.append(key) + return valid_indicators diff --git a/workers/ohsome_quality_analyst/indicators/minimal/indicator.py b/workers/ohsome_quality_analyst/indicators/minimal/indicator.py index 9c9835bb0..0373215a6 100644 --- a/workers/ohsome_quality_analyst/indicators/minimal/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/minimal/indicator.py @@ -10,7 +10,9 @@ class Minimal(BaseIndicator): - def __init__(self, layer: Layer, feature: Feature, thresholds: tuple) -> None: + def __init__( + self, layer: Layer, feature: Feature, thresholds: tuple = None + ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.count = 0 diff --git a/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml b/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml index 3037c6014..c56aaa7c4 100644 --- a/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml @@ -20,5 +20,5 @@ Minimal: # The first element is the threshold between the result class 1 # (maps to the label 'red'). - { layer: minimal, thresholds: [0.2, 0.4, 0.6, 0.8] } - # Thresholds can also be functins parameters + # Thresholds can also be function parameters # thresholds: [ {a: 0.2, b: 0.4,} { a: 0.6, b: 0.8 } ... ] diff --git a/workers/tests/integrationtests/test_oqt.py b/workers/tests/integrationtests/test_oqt.py index 7789b78ba..10d57eff3 100644 --- a/workers/tests/integrationtests/test_oqt.py +++ b/workers/tests/integrationtests/test_oqt.py @@ -159,6 +159,34 @@ def test_create_all_indicators(self): ) ) + @oqt_vcr.use_cassette() + def test_create_all_indicators_valid_layer(self): + with mock.patch( + "ohsome_quality_analyst.geodatabase.client.get_feature_ids", + new_callable=AsyncMock, + ) as get_feature_ids_mock: + get_feature_ids_mock.return_value = ["3", "12", "3", "12", "3", "12"] + asyncio.run( + oqt.create_all_indicators( + dataset="regions", + indicator_name="Minimal", + ) + ) + + @oqt_vcr.use_cassette() + def test_create_all_indicators_valid_indicators(self): + with mock.patch( + "ohsome_quality_analyst.geodatabase.client.get_feature_ids", + new_callable=AsyncMock, + ) as get_feature_ids_mock: + get_feature_ids_mock.return_value = ["3", "12", "3", "12", "3", "12"] + asyncio.run( + oqt.create_all_indicators( + dataset="regions", + layer_key="minimal", + ) + ) + def test_check_area_size(self): path = os.path.join( os.path.dirname(os.path.abspath(__file__)), "fixtures", "europe.geojson" From ef7e42d3c5d566d6aa7db36f34490a35f84eb7ba Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Mon, 26 Sep 2022 22:21:02 +0200 Subject: [PATCH 04/14] fix --- workers/ohsome_quality_analyst/api/request_models.py | 8 ++++---- workers/ohsome_quality_analyst/base/indicator.py | 4 +++- .../indicators/currentness/indicator.py | 2 ++ .../ghs_pop_comparison_buildings/indicator.py | 6 ++++-- .../indicators/ghs_pop_comparison_roads/indicator.py | 6 ++++-- .../indicators/mapping_saturation/indicator.py | 3 ++- .../indicators/poi_density/indicator.py | 6 ++++-- .../indicators/tags_ratio/indicator.py | 6 ++++-- workers/ohsome_quality_analyst/oqt.py | 10 ++++++---- .../test_indicator_mapping_saturation.py | 5 ++++- 10 files changed, 37 insertions(+), 19 deletions(-) diff --git a/workers/ohsome_quality_analyst/api/request_models.py b/workers/ohsome_quality_analyst/api/request_models.py index d3f521d77..2f292de67 100644 --- a/workers/ohsome_quality_analyst/api/request_models.py +++ b/workers/ohsome_quality_analyst/api/request_models.py @@ -36,14 +36,14 @@ class BaseIndicator(BaseModel): name: IndicatorEnum = pydantic.Field( ..., title="Indicator Name", example="GhsPopComparisonBuildings" ) - threshholds: Optional[ + thresholds: Optional[ Tuple[ Union[float, str], Union[str, float], Union[str, float], Union[str, float], ] - ] = None + ] include_svg: bool = False include_html: bool = False include_data: bool = False @@ -59,9 +59,9 @@ class Config: @pydantic.root_validator @classmethod def validate_thresholds(cls, values): - if values["threshholds"] is not None and values["name"] != "Currentness": + if values["thresholds"] is not None and values["name"] != "Currentness": raise ValueError( - "Setting custom threshholds is only supported for the Currentness " + "Setting custom thresholds is only supported for the Currentness " + "Indicator.", ) else: diff --git a/workers/ohsome_quality_analyst/base/indicator.py b/workers/ohsome_quality_analyst/base/indicator.py index 8ee05fb47..60f5b1bf6 100644 --- a/workers/ohsome_quality_analyst/base/indicator.py +++ b/workers/ohsome_quality_analyst/base/indicator.py @@ -86,7 +86,9 @@ def __init__( layer_thresholds = metadata.pop("layer-thresholds") if thresholds is None: # TODO: filter layer_thresholds - self.thresholds = layer_thresholds[layer.key] + for comb in layer_thresholds: + if comb["layer"] == layer.key: + self.thresholds = comb["thresholds"] else: self.thresholds = thresholds self.metadata: Metadata = from_dict(data_class=Metadata, data=metadata) diff --git a/workers/ohsome_quality_analyst/indicators/currentness/indicator.py b/workers/ohsome_quality_analyst/indicators/currentness/indicator.py index 20140a22d..d694ebab9 100644 --- a/workers/ohsome_quality_analyst/indicators/currentness/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/currentness/indicator.py @@ -1,6 +1,7 @@ import logging from io import StringIO from string import Template +from typing import Optional, Tuple import dateutil.parser import geojson @@ -33,6 +34,7 @@ def __init__( self, layer: Layer, feature: geojson.Feature, + thresholds: Optional[Tuple[float, float, float, float]], ) -> None: super().__init__(layer=layer, feature=feature) self.threshold_4 = 1 diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py index 2693048dd..44571e4f8 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py @@ -18,8 +18,10 @@ class GhsPopComparisonBuildings(BaseIndicator): """Set number of features and population into perspective.""" - def __init__(self, layer: Layer, feature: Feature) -> None: - super().__init__(layer=layer, feature=feature) + def __init__( + self, layer: Layer, feature: Feature, thresholds: tuple = None + ) -> None: + super().__init__(layer=layer, feature=feature, thresholds=thresholds) # Those attributes will be set during lifecycle of the object. self.pop_count = None self.area = None diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py index 7a208a42b..40778c28e 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py @@ -18,8 +18,10 @@ class GhsPopComparisonRoads(BaseIndicator): """Set number of features and population into perspective.""" - def __init__(self, layer: Layer, feature: Feature) -> None: - super().__init__(layer=layer, feature=feature) + def __init__( + self, layer: Layer, feature: Feature, thresholds: tuple = None + ) -> None: + super().__init__(layer=layer, feature=feature, thresholds=thresholds) # Those attributes will be set during lifecycle of the object. self.pop_count = None self.area = None diff --git a/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py b/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py index 033afb839..6a443d374 100644 --- a/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py @@ -1,7 +1,7 @@ import logging from io import StringIO from string import Template -from typing import List, Optional, Union +from typing import List, Optional, Tuple, Union import matplotlib.pyplot as plt import numpy as np @@ -40,6 +40,7 @@ def __init__( self, layer: Layer, feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]], time_range: str = "2008-01-01//P1M", ) -> None: super().__init__(layer=layer, feature=feature) diff --git a/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py b/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py index eb99d02b2..a9a2b78ba 100644 --- a/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py @@ -17,8 +17,10 @@ class PoiDensity(BaseIndicator): - def __init__(self, layer: Layer, feature: Feature) -> None: - super().__init__(layer=layer, feature=feature) + def __init__( + self, layer: Layer, feature: Feature, thresholds: tuple = None + ) -> None: + super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.threshold_yellow = 30 self.threshold_red = 10 self.area_sqkm = None diff --git a/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py b/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py index 244129bbf..d53b6fede 100644 --- a/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py @@ -13,8 +13,10 @@ class TagsRatio(BaseIndicator): - def __init__(self, layer: Layer, feature: Feature) -> None: - super().__init__(layer=layer, feature=feature) + def __init__( + self, layer: Layer, feature: Feature, thresholds: tuple = None + ) -> None: + super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.threshold_yellow = 0.75 self.threshold_red = 0.25 self.count_all = None diff --git a/workers/ohsome_quality_analyst/oqt.py b/workers/ohsome_quality_analyst/oqt.py index 9e01a2d06..7688471d6 100644 --- a/workers/ohsome_quality_analyst/oqt.py +++ b/workers/ohsome_quality_analyst/oqt.py @@ -167,7 +167,9 @@ async def _( feature_id = parameters.feature_id feature = await db_client.get_feature_from_db(dataset, feature_id) indicator_class = name_to_class(class_type="indicator", name=name) - indicator_raw = indicator_class(layer=layer, feature=feature) + indicator_raw = indicator_class( + layer=layer, feature=feature, thresholds=parameters.thresholds + ) failure = False try: indicator = await db_client.load_indicator_results( @@ -182,7 +184,7 @@ async def _( IndicatorBpolys( name=name, layerKey=parameters.layer_key.value, - threshholds=parameters.threshholds, + thresholds=parameters.thresholds, bpolys=feature, ) ) @@ -200,7 +202,7 @@ async def _( name = parameters.name.value layer: Layer = get_layer_definition(parameters.layer_key.value) feature = parameters.bpolys - threshholds = parameters.threshholds + thresholds = parameters.thresholds logging.info("Calculating Indicator for custom AOI ...") logging.info("Feature id: {0:4}".format(feature.get("id", 1))) @@ -208,7 +210,7 @@ async def _( logging.info("Layer name: {0:4}".format(layer.name)) indicator_class = name_to_class(class_type="indicator", name=name) - indicator = indicator_class(layer, feature, threshholds) + indicator = indicator_class(layer, feature, thresholds) logging.info("Run preprocessing") await indicator.preprocess() diff --git a/workers/tests/integrationtests/test_indicator_mapping_saturation.py b/workers/tests/integrationtests/test_indicator_mapping_saturation.py index 6c1fb1294..734e44b61 100644 --- a/workers/tests/integrationtests/test_indicator_mapping_saturation.py +++ b/workers/tests/integrationtests/test_indicator_mapping_saturation.py @@ -19,7 +19,9 @@ def setUp(self): @oqt_vcr.use_cassette() def test(self): # Heidelberg - indicator = MappingSaturation(layer=self.layer, feature=self.feature) + indicator = MappingSaturation( + layer=self.layer, feature=self.feature, thresholds=None + ) self.assertIsNotNone(indicator.attribution()) asyncio.run(indicator.preprocess()) @@ -91,6 +93,7 @@ def test_immutable_attribute(self): indicator = MappingSaturation( layer=get_layer_fixture("building_count"), feature=feature, + thresholds=None, ) asyncio.run(indicator.preprocess()) indicator.calculate() From 24605930088a5d1d903de9e95137f41541002d2b Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Tue, 27 Sep 2022 14:10:46 +0200 Subject: [PATCH 05/14] fix: tests --- workers/ohsome_quality_analyst/base/indicator.py | 1 - workers/tests/integrationtests/test_api.py | 7 ------- .../integrationtests/test_api_indicator_geojson_io.py | 3 +++ workers/tests/integrationtests/test_cli.py | 8 -------- .../test_indicator_building_completeness.py | 4 +++- .../tests/integrationtests/test_indicator_currentness.py | 5 ++++- .../test_indicator_ghs_pop_comparison_buildings.py | 4 +++- .../test_indicator_ghs_pop_comparison_roads.py | 1 + workers/tests/integrationtests/test_indicator_minimal.py | 2 +- .../tests/integrationtests/test_indicator_poi_density.py | 4 +++- .../tests/integrationtests/test_indicator_tags_ratio.py | 1 + workers/tests/integrationtests/test_oqt.py | 1 + workers/tests/unittests/test_api_request_models.py | 3 +++ 13 files changed, 23 insertions(+), 21 deletions(-) diff --git a/workers/ohsome_quality_analyst/base/indicator.py b/workers/ohsome_quality_analyst/base/indicator.py index 60f5b1bf6..67e964c03 100644 --- a/workers/ohsome_quality_analyst/base/indicator.py +++ b/workers/ohsome_quality_analyst/base/indicator.py @@ -85,7 +85,6 @@ def __init__( metadata = get_metadata("indicators", type(self).__name__) layer_thresholds = metadata.pop("layer-thresholds") if thresholds is None: - # TODO: filter layer_thresholds for comb in layer_thresholds: if comb["layer"] == layer.key: self.thresholds = comb["thresholds"] diff --git a/workers/tests/integrationtests/test_api.py b/workers/tests/integrationtests/test_api.py index 9095fc3f7..6302981af 100644 --- a/workers/tests/integrationtests/test_api.py +++ b/workers/tests/integrationtests/test_api.py @@ -50,13 +50,6 @@ def test_get_available_regions(self): for region in response_content["result"]: self.assertIsInstance(region, dict) - def test_list_indicator_layer_combinations(self): - url = "/indicator-layer-combinations" - response = self.client.get(url) - self.assertEqual(response.status_code, 200) - - self.assertIsInstance(response.json(), dict) - def test_list_indicators(self): url = "/indicators" response = self.client.get(url) diff --git a/workers/tests/integrationtests/test_api_indicator_geojson_io.py b/workers/tests/integrationtests/test_api_indicator_geojson_io.py index e7c0e4fb1..367ee2258 100644 --- a/workers/tests/integrationtests/test_api_indicator_geojson_io.py +++ b/workers/tests/integrationtests/test_api_indicator_geojson_io.py @@ -96,6 +96,7 @@ def test_invalid_set_of_arguments(self): "bpolys": bpolys, "dataset": "foo", "featureId": "3", + "thresholds": None, } response = self.client.post(self.endpoint, json=parameters) self.assertEqual(response.status_code, 422) @@ -195,6 +196,7 @@ def test_indicator_layer_data(self): ] }, }, + "thresholds": None, } response = self.client.post(self.endpoint, json=parameters) self.run_tests(response, (self.general_schema, self.feature_schema)) @@ -208,6 +210,7 @@ def test_indicator_layer_data_invalid(self): "description": "", "data": {"result": [{"value": 1.0}]}, # Missing timestamp item }, + "thresholds": None, } response = self.client.post(self.endpoint, json=parameters) self.assertEqual(response.status_code, 422) diff --git a/workers/tests/integrationtests/test_cli.py b/workers/tests/integrationtests/test_cli.py index d23baeb61..fa36a14ba 100644 --- a/workers/tests/integrationtests/test_cli.py +++ b/workers/tests/integrationtests/test_cli.py @@ -126,14 +126,6 @@ def test_get_available_regions(self): self.assertEqual(result.exit_code, 0) self.assertTrue(result.output, str) - def test_get_indicator_layer_combination(self): - result = self.runner.invoke( - cli, - ["-q", "list-indicator-layer-combination"], - ) - self.assertEqual(result.exit_code, 0) - self.assertIsInstance(result.output, str) - def test_list_indicators(self): result = self.runner.invoke( cli, diff --git a/workers/tests/integrationtests/test_indicator_building_completeness.py b/workers/tests/integrationtests/test_indicator_building_completeness.py index 34c01c021..065640959 100644 --- a/workers/tests/integrationtests/test_indicator_building_completeness.py +++ b/workers/tests/integrationtests/test_indicator_building_completeness.py @@ -24,7 +24,9 @@ def setUp(self): db_client.get_feature_from_db(dataset="regions", feature_id="12") ) self.layer = get_layer_fixture("building_area") - self.indicator = BuildingCompleteness(feature=self.feature, layer=self.layer) + self.indicator = BuildingCompleteness( + feature=self.feature, layer=self.layer, thresholds=None + ) @mock.patch("ohsome_quality_analyst.raster.client.get_config_value") @oqt_vcr.use_cassette() diff --git a/workers/tests/integrationtests/test_indicator_currentness.py b/workers/tests/integrationtests/test_indicator_currentness.py index c7c46afcb..614804c82 100644 --- a/workers/tests/integrationtests/test_indicator_currentness.py +++ b/workers/tests/integrationtests/test_indicator_currentness.py @@ -26,6 +26,7 @@ def test(self): indicator = Currentness( feature=feature, layer=get_layer_fixture("major_roads_count"), + thresholds=None, ) self.assertIsNotNone(indicator.attribution()) @@ -51,7 +52,9 @@ def test_no_amenities(self): with open(infile, "r") as f: feature = geojson.load(f) - indicator = Currentness(feature=feature, layer=get_layer_fixture("amenities")) + indicator = Currentness( + feature=feature, layer=get_layer_fixture("amenities"), thresholds=None + ) asyncio.run(indicator.preprocess()) self.assertEqual(indicator.element_count, 0) diff --git a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py index 91a317b6f..05b4fc8c5 100644 --- a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py +++ b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py @@ -17,7 +17,9 @@ def setUp(self): db_client.get_feature_from_db(dataset="regions", feature_id="3") ) layer = get_layer_fixture("building_count") - self.indicator = GhsPopComparisonBuildings(feature=feature, layer=layer) + self.indicator = GhsPopComparisonBuildings( + feature=feature, layer=layer, thresholds=None + ) @oqt_vcr.use_cassette() def test(self): diff --git a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py index d3b4adb1f..e0ad87565 100644 --- a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py +++ b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py @@ -19,6 +19,7 @@ def setUp(self): self.indicator = GhsPopComparisonRoads( feature=feature, layer=get_layer_fixture("major_roads_length"), + thresholds=None, ) @oqt_vcr.use_cassette() diff --git a/workers/tests/integrationtests/test_indicator_minimal.py b/workers/tests/integrationtests/test_indicator_minimal.py index 62a9b6848..15906ce82 100644 --- a/workers/tests/integrationtests/test_indicator_minimal.py +++ b/workers/tests/integrationtests/test_indicator_minimal.py @@ -10,7 +10,7 @@ class TestIndicatorMinimal(unittest.TestCase): def setUp(self): feature = get_geojson_fixture("heidelberg-altstadt-feature.geojson") layer = get_layer_fixture("minimal") - self.indicator = Minimal(feature=feature, layer=layer) + self.indicator = Minimal(feature=feature, layer=layer, thresholds=None) @oqt_vcr.use_cassette() def test(self): diff --git a/workers/tests/integrationtests/test_indicator_poi_density.py b/workers/tests/integrationtests/test_indicator_poi_density.py index 72fffb10b..c5b71805f 100644 --- a/workers/tests/integrationtests/test_indicator_poi_density.py +++ b/workers/tests/integrationtests/test_indicator_poi_density.py @@ -19,7 +19,9 @@ def setUp(self): ) with open(infile, "r") as f: feature = geojson.load(f) - self.indicator = PoiDensity(feature=feature, layer=get_layer_fixture("poi")) + self.indicator = PoiDensity( + feature=feature, layer=get_layer_fixture("poi"), thresholds=None + ) @oqt_vcr.use_cassette() def test(self): diff --git a/workers/tests/integrationtests/test_indicator_tags_ratio.py b/workers/tests/integrationtests/test_indicator_tags_ratio.py index 06b875b43..f9cc69a93 100644 --- a/workers/tests/integrationtests/test_indicator_tags_ratio.py +++ b/workers/tests/integrationtests/test_indicator_tags_ratio.py @@ -23,6 +23,7 @@ def test(self): indicator = TagsRatio( feature=self.feature, layer=get_layer_fixture("jrc_health_count"), + thresholds=None, ) self.assertIsNotNone(indicator.attribution()) diff --git a/workers/tests/integrationtests/test_oqt.py b/workers/tests/integrationtests/test_oqt.py index 10d57eff3..6ca02b136 100644 --- a/workers/tests/integrationtests/test_oqt.py +++ b/workers/tests/integrationtests/test_oqt.py @@ -235,6 +235,7 @@ def test_create_indicator_as_geojson_size_limit_layer_data(self): ] }, }, + thresholds=None, ) asyncio.run(oqt.create_indicator_as_geojson(parameters, size_restriction=True)) diff --git a/workers/tests/unittests/test_api_request_models.py b/workers/tests/unittests/test_api_request_models.py index 5a74dc2f8..f22842282 100644 --- a/workers/tests/unittests/test_api_request_models.py +++ b/workers/tests/unittests/test_api_request_models.py @@ -202,6 +202,7 @@ def test_invalid_set_of_arguments(self): "layerKey": "minimal", "dataset": "regions", "featureId": "3", + "thresholds": None, }, { "name": "Minimal", @@ -209,11 +210,13 @@ def test_invalid_set_of_arguments(self): "dataset": "regions", "featureId": "3", "fidField": "ogc_fid", + "thresholds": None, }, { "name": "Minimal", "layerKey": "minimal", "bpolys": self.bpolys, + "thresholds": None, }, ) for combination in all_combinations: From 56cd3521d0ff4232f45f470fd4c33de0d5070761 Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Wed, 12 Oct 2022 17:09:38 +0200 Subject: [PATCH 06/14] fix: --- workers/ohsome_quality_analyst/base/indicator.py | 2 ++ .../indicators/mapping_saturation/metadata.yaml | 1 + .../integrationtests/test_indicator_mapping_saturation.py | 6 +++++- 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/workers/ohsome_quality_analyst/base/indicator.py b/workers/ohsome_quality_analyst/base/indicator.py index 67e964c03..9042c379b 100644 --- a/workers/ohsome_quality_analyst/base/indicator.py +++ b/workers/ohsome_quality_analyst/base/indicator.py @@ -90,6 +90,8 @@ def __init__( self.thresholds = comb["thresholds"] else: self.thresholds = thresholds + # default threshold for self-defined layer + self.metadata: Metadata = from_dict(data_class=Metadata, data=metadata) self.result: Result = Result( description=self.metadata.label_description["undefined"], diff --git a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml index 6182d9fa0..9d70d8a9d 100644 --- a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml @@ -16,6 +16,7 @@ MappingSaturation: result_description: | The saturation of the last 3 years is $saturation%. layer-thresholds: + - { layer: null, thresholds: [0.3, null, 0.97, null]} - { layer: amenities, thresholds: [0.3, null, 0.97, null] } - { layer: building_count, thresholds: [0.3, null, 0.97, null] } - { layer: infrastructure_lines, thresholds: [0.3, null, 0.97, null] } diff --git a/workers/tests/integrationtests/test_indicator_mapping_saturation.py b/workers/tests/integrationtests/test_indicator_mapping_saturation.py index 734e44b61..6086eb30a 100644 --- a/workers/tests/integrationtests/test_indicator_mapping_saturation.py +++ b/workers/tests/integrationtests/test_indicator_mapping_saturation.py @@ -50,7 +50,11 @@ def test(self): @oqt_vcr.use_cassette() def test_as_feature(self): - indicator = MappingSaturation(layer=self.layer, feature=self.feature) + indicator = MappingSaturation( + layer=self.layer, + feature=self.feature, + thresholds=None, + ) asyncio.run(indicator.preprocess()) indicator.calculate() From 5e4f53fcc629e03c9e12b46cbac57867197cf77a Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Sun, 16 Oct 2022 15:27:16 +0200 Subject: [PATCH 07/14] fix: --- .../ohsome_quality_analyst/api/request_models.py | 14 ++++++++++---- workers/ohsome_quality_analyst/base/indicator.py | 2 -- workers/ohsome_quality_analyst/oqt.py | 2 +- workers/tests/unittests/test_definitions.py | 8 ++++---- workers/tests/unittests/test_load_metadata.py | 1 + 5 files changed, 16 insertions(+), 11 deletions(-) diff --git a/workers/ohsome_quality_analyst/api/request_models.py b/workers/ohsome_quality_analyst/api/request_models.py index 2f292de67..2f080f2f9 100644 --- a/workers/ohsome_quality_analyst/api/request_models.py +++ b/workers/ohsome_quality_analyst/api/request_models.py @@ -150,8 +150,11 @@ class IndicatorBpolys(BaseIndicator, BaseLayerName, BaseBpolys): @pydantic.root_validator @classmethod def validate_indicator_layer(cls, values): - indicator_key = values["name"].value - layer_key = values["layer_key"].value + try: + indicator_key = values["name"].value + layer_key = values["layer_key"].value + except KeyError: + raise ValueError("An issue with the layer or indicator name occurred.") if layer_key not in get_valid_layers(indicator_key): raise ValueError( "Layer ({0}) is not available for indicator ({1})".format( @@ -166,8 +169,11 @@ class IndicatorDatabase(BaseIndicator, BaseLayerName, BaseDatabase): @pydantic.root_validator @classmethod def validate_indicator_layer(cls, values): - indicator_key = values["name"].value - layer_key = values["layer_key"].value + try: + indicator_key = values["name"].value + layer_key = values["layer_key"].value + except KeyError: + raise ValueError("An issue with the layer or indicator name occurred.") if layer_key not in get_valid_layers(indicator_key): raise ValueError( "Layer ({0}) is not available for indicator ({1})".format( diff --git a/workers/ohsome_quality_analyst/base/indicator.py b/workers/ohsome_quality_analyst/base/indicator.py index 9042c379b..67e964c03 100644 --- a/workers/ohsome_quality_analyst/base/indicator.py +++ b/workers/ohsome_quality_analyst/base/indicator.py @@ -90,8 +90,6 @@ def __init__( self.thresholds = comb["thresholds"] else: self.thresholds = thresholds - # default threshold for self-defined layer - self.metadata: Metadata = from_dict(data_class=Metadata, data=metadata) self.result: Result = Result( description=self.metadata.label_description["undefined"], diff --git a/workers/ohsome_quality_analyst/oqt.py b/workers/ohsome_quality_analyst/oqt.py index 7688471d6..b23cb1fe1 100644 --- a/workers/ohsome_quality_analyst/oqt.py +++ b/workers/ohsome_quality_analyst/oqt.py @@ -239,7 +239,7 @@ async def _( logging.info("Layer name: {0:4}".format(layer.name)) indicator_class = name_to_class(class_type="indicator", name=name) - indicator = indicator_class(layer, feature) + indicator = indicator_class(layer, feature, thresholds=None) logging.info("Run preprocessing") await indicator.preprocess() diff --git a/workers/tests/unittests/test_definitions.py b/workers/tests/unittests/test_definitions.py index 6f4afdc1f..5e848d673 100644 --- a/workers/tests/unittests/test_definitions.py +++ b/workers/tests/unittests/test_definitions.py @@ -75,17 +75,17 @@ def test_get_valid_indicators(self): indicators = definitions.get_valid_indicators("building_count") self.assertEqual( indicators, - ( - "GhsPopComparisonBuildings", + [ "MappingSaturation", + "GhsPopComparisonBuildings", "Currentness", "TagsRatio", - ), + ], ) def test_get_valid_layers(self): layers = definitions.get_valid_layers("Minimal") self.assertEqual( layers, - ("minimal",), + ["minimal"], ) diff --git a/workers/tests/unittests/test_load_metadata.py b/workers/tests/unittests/test_load_metadata.py index d5b947c84..5106613b6 100644 --- a/workers/tests/unittests/test_load_metadata.py +++ b/workers/tests/unittests/test_load_metadata.py @@ -19,6 +19,7 @@ def test_validate_indicator_schema(self): "undefined": str, }, "result_description": str, + "layer-thresholds": [dict], } } ) From b9a50808895982f573ad342ff7d74d8ed84af452 Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 18 Oct 2022 14:11:54 +0200 Subject: [PATCH 08/14] test: fix test by reverting changes --- workers/tests/unittests/test_api_request_models.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/workers/tests/unittests/test_api_request_models.py b/workers/tests/unittests/test_api_request_models.py index f22842282..5a74dc2f8 100644 --- a/workers/tests/unittests/test_api_request_models.py +++ b/workers/tests/unittests/test_api_request_models.py @@ -202,7 +202,6 @@ def test_invalid_set_of_arguments(self): "layerKey": "minimal", "dataset": "regions", "featureId": "3", - "thresholds": None, }, { "name": "Minimal", @@ -210,13 +209,11 @@ def test_invalid_set_of_arguments(self): "dataset": "regions", "featureId": "3", "fidField": "ogc_fid", - "thresholds": None, }, { "name": "Minimal", "layerKey": "minimal", "bpolys": self.bpolys, - "thresholds": None, }, ) for combination in all_combinations: From 2e8f27c8b300c07aadf092ac5a7c39ca54e82dd3 Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 18 Oct 2022 14:20:35 +0200 Subject: [PATCH 09/14] exclude default layer (None) from valid layersis `get_valid_layer()` result will filter out None values. --- workers/ohsome_quality_analyst/definitions.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/workers/ohsome_quality_analyst/definitions.py b/workers/ohsome_quality_analyst/definitions.py index 85fb4fd3e..7b1484f0b 100644 --- a/workers/ohsome_quality_analyst/definitions.py +++ b/workers/ohsome_quality_analyst/definitions.py @@ -219,7 +219,6 @@ def get_attribution(data_keys: list) -> str: return "; ".join([str(v) for v in filtered.values()]) -# TODO def get_valid_layers(indicator_name: str) -> list: """Get valid Indicator/Layer combination of an indicator.""" indicator_layer = load_metadata("indicators") @@ -228,11 +227,10 @@ def get_valid_layers(indicator_name: str) -> list: if key == indicator_name: for comb in value["layer-thresholds"]: valid_layers.append(comb["layer"]) - return valid_layers + return [l for l in valid_layers if l is not None] # noqa: E741 -# TODO -def get_valid_indicators(layer_key: str) -> tuple: +def get_valid_indicators(layer_key: str) -> list: """Get valid Indicator/Layer combination of a layer.""" indicator_layer = load_metadata("indicators") valid_indicators = [] From e65c5a356791bda1aee42044ab92e01382ecda56 Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 18 Oct 2022 14:52:34 +0200 Subject: [PATCH 10/14] get thresholds from args or parse from metadata --- workers/ohsome_quality_analyst/base/indicator.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/workers/ohsome_quality_analyst/base/indicator.py b/workers/ohsome_quality_analyst/base/indicator.py index 67e964c03..98d76274f 100644 --- a/workers/ohsome_quality_analyst/base/indicator.py +++ b/workers/ohsome_quality_analyst/base/indicator.py @@ -83,13 +83,15 @@ def __init__( # setattr(object, key, value) could be used instead of relying on from_dict. metadata = get_metadata("indicators", type(self).__name__) + layer_thresholds = metadata.pop("layer-thresholds") - if thresholds is None: - for comb in layer_thresholds: - if comb["layer"] == layer.key: - self.thresholds = comb["thresholds"] - else: + if thresholds is not None: self.thresholds = thresholds + elif layer_thresholds[layer.key] is not None: + self.thresholds = layer_thresholds[layer.key] + else: + self.thresholds = layer_thresholds["default"] + self.metadata: Metadata = from_dict(data_class=Metadata, data=metadata) self.result: Result = Result( description=self.metadata.label_description["undefined"], From c017dbebd2d0831cc2a77b068b105a9ddf456ae0 Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 18 Oct 2022 14:53:22 +0200 Subject: [PATCH 11/14] get valid indicators/layers from metadata --- workers/ohsome_quality_analyst/definitions.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/workers/ohsome_quality_analyst/definitions.py b/workers/ohsome_quality_analyst/definitions.py index 7b1484f0b..322662c74 100644 --- a/workers/ohsome_quality_analyst/definitions.py +++ b/workers/ohsome_quality_analyst/definitions.py @@ -221,20 +221,16 @@ def get_attribution(data_keys: list) -> str: def get_valid_layers(indicator_name: str) -> list: """Get valid Indicator/Layer combination of an indicator.""" - indicator_layer = load_metadata("indicators") - valid_layers = [] - for key, value in indicator_layer.items(): - if key == indicator_name: - for comb in value["layer-thresholds"]: - valid_layers.append(comb["layer"]) - return [l for l in valid_layers if l is not None] # noqa: E741 + metadata = load_metadata("indicators") + layers = metadata[indicator_name]["layer-thresholds"].keys() + return [l for l in layers if l != "default"] # noqa: E741 def get_valid_indicators(layer_key: str) -> list: """Get valid Indicator/Layer combination of a layer.""" - indicator_layer = load_metadata("indicators") + metadata = load_metadata("indicators") valid_indicators = [] - for key, value in indicator_layer.items(): - if any(comb["layer"] == layer_key for comb in value["layer-thresholds"]): - valid_indicators.append(key) + for indicator_key, metadata_ in metadata.items(): + if any(layer_key in metadata_["layer-thresholds"].keys()): + valid_indicators.append(indicator_key) return valid_indicators From d170881fa60678389850055d299bf38cb6b4d8e8 Mon Sep 17 00:00:00 2001 From: Matthias Schaub Date: Tue, 18 Oct 2022 15:07:10 +0200 Subject: [PATCH 12/14] change layer/threshold list in metadata --- .../indicators/building_completeness/metadata.yaml | 3 ++- .../indicators/currentness/metadata.yaml | 13 +++++++------ .../ghs_pop_comparison_buildings/metadata.yaml | 3 ++- .../ghs_pop_comparison_roads/metadata.yaml | 3 ++- .../indicators/mapping_saturation/metadata.yaml | 14 +++++++------- .../indicators/minimal/metadata.yaml | 13 ++++++------- .../indicators/poi_density/metadata.yaml | 3 ++- .../indicators/tags_ratio/metadata.yaml | 5 +++-- 8 files changed, 31 insertions(+), 26 deletions(-) diff --git a/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml b/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml index 2856c8843..7e708bdae 100644 --- a/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/building_completeness/metadata.yaml @@ -24,4 +24,5 @@ BuildingCompleteness: predicted building area is $completeness_ratio %. The weight is the predicted building area. layer-thresholds: - - { layer: building_area, thresholds: [0.2, 0.5, 0.8, 0.9] } + default: [0.2, 0.5, 0.8, 0.9] + building_area: null diff --git a/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml b/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml index e03ac7ce0..3e508c966 100644 --- a/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml @@ -21,9 +21,10 @@ Currentness: result_description: | Over 50% of the $elements features ($layer_name) were edited $years. layer-thresholds: - - { layer: amenities, thresholds: [ 0.2, null, 0.6, null] } - - { layer: building_count, thresholds: [ 0.2, null, 0.6, null] } - - { layer: major_roads_count, thresholds: [ 0.2, null, 0.6, null] } - - { layer: infrastructure_lines, thresholds: [ 0.2, null, 0.6, null] } - - { layer: poi, thresholds: [ 0.2, null, 0.6, null] } - - { layer: lulc, thresholds: [ 0.2, null, 0.6, null] } + default: [ 0.2, null, 0.6, null] + amenities: null + building_count: null + major_roads_count: null + infrastructure_lines: null + poi: null + lulc: null diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml index b655ea660..774b9df3e 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/metadata.yaml @@ -25,4 +25,5 @@ GhsPopComparisonBuildings: $pop_count_per_sqkm of people per sqkm. $feature_count_per_sqkm buildings per sqkm mapped. layer-thresholds: - - { layer: building_count, thresholds: [{ a:0.75 }, null, { a: 5.0 }, null] } + default: [{ a: 0.75 }, null, { a: 5.0 }, null] + building_count: null diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml index aec0b4f12..840beae7d 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml @@ -25,4 +25,5 @@ GhsPopComparisonRoads: $pop_count_per_sqkm of people per sqkm. $feature_length_per_sqkm km of roads per sqkm mapped. layer-thresholds: - - { layer: major_roads_length, thresholds: [{ a: 1000 }, null, { a: 500 }, null] } + default: [{ a: 1000 }, null, { a: 500 }, null] + major_roads_length: null diff --git a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml index 9d70d8a9d..8a9f342b8 100644 --- a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml @@ -16,10 +16,10 @@ MappingSaturation: result_description: | The saturation of the last 3 years is $saturation%. layer-thresholds: - - { layer: null, thresholds: [0.3, null, 0.97, null]} - - { layer: amenities, thresholds: [0.3, null, 0.97, null] } - - { layer: building_count, thresholds: [0.3, null, 0.97, null] } - - { layer: infrastructure_lines, thresholds: [0.3, null, 0.97, null] } - - { layer: lulc, thresholds: [0.3, null, 0.97, null] } - - { layer: major_roads_length, thresholds: [0.3, null, 0.97, null] } - - { layer: poi, thresholds: [0.3, null, 0.97, null] } + default: [0.3, null, 0.97, null] + amenities: null + building_count: null + infrastructure_lines: null + lulc: null + major_roads_length: null + poi, thresholds: null diff --git a/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml b/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml index c56aaa7c4..503af7c40 100644 --- a/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/minimal/metadata.yaml @@ -15,10 +15,9 @@ Minimal: result_description: | Some description of the result. layer-thresholds: - # List of associative lists of valid layers and thresholds. - # A threshold is a list of four float values. - # The first element is the threshold between the result class 1 - # (maps to the label 'red'). - - { layer: minimal, thresholds: [0.2, 0.4, 0.6, 0.8] } - # Thresholds can also be function parameters - # thresholds: [ {a: 0.2, b: 0.4,} { a: 0.6, b: 0.8 } ... ] + # layer-key: [thresholds] + # A threshold is a list of four values (floats) or function parameters (associative list). + # The first element is the threshold between the result class 1 (maps to the label 'red'). + # The layer key "None" signifies the default thresholds. + default: [0.3, null, 0.97, null] + minimal: null diff --git a/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml b/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml index c9da5bdba..fd67874c9 100644 --- a/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/poi_density/metadata.yaml @@ -23,4 +23,5 @@ PoiDensity: (points of reference, e.g. waterbodies, supermarkets, churches, bus stops) is $result features per sqkm. layer-thresholds: - - { layer: poi, thresholds: [10, null, null, 30] } + default: [10, null, null, 30] + poi: null diff --git a/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml b/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml index 7a9f7f5b0..f7e88a622 100644 --- a/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml @@ -18,5 +18,6 @@ TagsRatio: The ratio of the features (all: $all) compared to features with expected tags (matched: $matched) is $result. layer-thresholds: - - { layer: building_count, thresholds: [ 0.25, null, 0.75, null] } - - { layer: major_roads_length, thresholds: [ 0.25, null, 0.75, null] } + default: [ 0.25, null, 0.75, null] + building_count: null + major_roads_length: null From 15c119ab2965baf1f36aac24c98ffa2364ada2fd Mon Sep 17 00:00:00 2001 From: Gigaszi Date: Wed, 19 Oct 2022 14:21:32 +0200 Subject: [PATCH 13/14] fix: thresholds for indicator initialization is None by default; add all indicator-layer-combinations --- .../building_completeness/indicator.py | 2 +- .../indicators/currentness/indicator.py | 10 ++++----- .../indicators/currentness/metadata.yaml | 22 ++++++++++++++++--- .../ghs_pop_comparison_buildings/indicator.py | 6 ++++- .../ghs_pop_comparison_roads/indicator.py | 6 ++++- .../ghs_pop_comparison_roads/metadata.yaml | 3 ++- .../mapping_saturation/indicator.py | 2 +- .../mapping_saturation/metadata.yaml | 12 +++++++++- .../indicators/minimal/indicator.py | 6 ++++- .../indicators/poi_density/indicator.py | 6 ++++- .../indicators/tags_ratio/indicator.py | 6 ++++- .../indicators/tags_ratio/metadata.yaml | 8 +++++++ workers/ohsome_quality_analyst/oqt.py | 2 +- .../test_api_indicator_geojson_io.py | 3 --- .../test_indicator_building_completeness.py | 4 +--- .../test_indicator_currentness.py | 5 +---- ..._indicator_ghs_pop_comparison_buildings.py | 4 +--- ...test_indicator_ghs_pop_comparison_roads.py | 1 - .../test_indicator_mapping_saturation.py | 11 ++-------- .../test_indicator_minimal.py | 2 +- .../test_indicator_poi_density.py | 4 +--- .../test_indicator_tags_ratio.py | 1 - workers/tests/integrationtests/test_oqt.py | 1 - 23 files changed, 80 insertions(+), 47 deletions(-) diff --git a/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py b/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py index 5adc29153..e661cbd36 100644 --- a/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/building_completeness/indicator.py @@ -59,7 +59,7 @@ def __init__( self, layer: Layer, feature: Feature, - thresholds: Optional[Tuple[float, float, float, float]], + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: if thresholds is None: thresholds = (0.2, 0.5, 0.8, 0.9) diff --git a/workers/ohsome_quality_analyst/indicators/currentness/indicator.py b/workers/ohsome_quality_analyst/indicators/currentness/indicator.py index c80fc079e..cda66daa5 100644 --- a/workers/ohsome_quality_analyst/indicators/currentness/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/currentness/indicator.py @@ -34,13 +34,13 @@ def __init__( self, layer: Layer, feature: geojson.Feature, - thresholds: Optional[Tuple[float, float, float, float]], + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: super().__init__(layer=layer, feature=feature) - self.threshold_4 = 2 - self.threshold_3 = 3 - self.threshold_2 = 4 - self.threshold_1 = 8 + self.threshold_4 = self.thresholds[0] + self.threshold_3 = self.thresholds[1] + self.threshold_2 = self.thresholds[2] + self.threshold_1 = self.thresholds[3] self.element_count = None self.contributions_sum = None self.contributions_rel = {} # yearly interval diff --git a/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml b/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml index 3e508c966..0d19f022d 100644 --- a/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/currentness/metadata.yaml @@ -21,10 +21,26 @@ Currentness: result_description: | Over 50% of the $elements features ($layer_name) were edited $years. layer-thresholds: - default: [ 0.2, null, 0.6, null] + default: [ 2, 3, 4, 8] amenities: null building_count: null - major_roads_count: null infrastructure_lines: null - poi: null + jrc_airport_count: null + jrc_bridge_count: null + jrc_cultural_heritage_site_count: null + jrc_education_count: null + jrc_health_count: null + jrc_mass_gathering_sites_count: null + jrc_power_generation_plant_count: null + jrc_railway_count: null + jrc_road_count: null + jrc_water_treatment_plant_count: null lulc: null + major_roads_count: null + mapaction_lakes_count: null + mapaction_major_roads_length: null + mapaction_rail_length: null + mapaction_rivers_length: null + mapaction_settlements_count: null + poi: null + diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py index 44571e4f8..2398179dd 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py @@ -1,6 +1,7 @@ import logging from io import StringIO from string import Template +from typing import Optional, Tuple import dateutil.parser import matplotlib.pyplot as plt @@ -19,7 +20,10 @@ class GhsPopComparisonBuildings(BaseIndicator): """Set number of features and population into perspective.""" def __init__( - self, layer: Layer, feature: Feature, thresholds: tuple = None + self, + layer: Layer, + feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) # Those attributes will be set during lifecycle of the object. diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py index 40778c28e..c9ac45f82 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py @@ -1,6 +1,7 @@ import logging from io import StringIO from string import Template +from typing import Optional, Tuple import dateutil.parser import matplotlib.pyplot as plt @@ -19,7 +20,10 @@ class GhsPopComparisonRoads(BaseIndicator): """Set number of features and population into perspective.""" def __init__( - self, layer: Layer, feature: Feature, thresholds: tuple = None + self, + layer: Layer, + feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) # Those attributes will be set during lifecycle of the object. diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml index 840beae7d..20af991d8 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/metadata.yaml @@ -26,4 +26,5 @@ GhsPopComparisonRoads: $feature_length_per_sqkm km of roads per sqkm mapped. layer-thresholds: default: [{ a: 1000 }, null, { a: 500 }, null] - major_roads_length: null + jrc_road_length: null + major_roads_length: null \ No newline at end of file diff --git a/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py b/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py index 6a443d374..bd1386c8f 100644 --- a/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/mapping_saturation/indicator.py @@ -40,7 +40,7 @@ def __init__( self, layer: Layer, feature: Feature, - thresholds: Optional[Tuple[float, float, float, float]], + thresholds: Optional[Tuple[float, float, float, float]] = None, time_range: str = "2008-01-01//P1M", ) -> None: super().__init__(layer=layer, feature=feature) diff --git a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml index 8a9f342b8..c57e67fef 100644 --- a/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/mapping_saturation/metadata.yaml @@ -20,6 +20,16 @@ MappingSaturation: amenities: null building_count: null infrastructure_lines: null + jrc_education_count: null + jrc_health_count: null + jrc_mass_gathering_sites_count: null + jrc_railway_length: null + jrc_road_length: null lulc: null major_roads_length: null - poi, thresholds: null + mapaction_lakes_area: null + mapaction_major_roads_length: null + mapaction_rail_length: null + mapaction_rivers_length: null + mapaction_settlements_count: null + poi: null diff --git a/workers/ohsome_quality_analyst/indicators/minimal/indicator.py b/workers/ohsome_quality_analyst/indicators/minimal/indicator.py index 0373215a6..76be13691 100644 --- a/workers/ohsome_quality_analyst/indicators/minimal/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/minimal/indicator.py @@ -1,5 +1,6 @@ """An Indicator for testing purposes.""" from string import Template +from typing import Optional, Tuple import dateutil.parser from geojson import Feature @@ -11,7 +12,10 @@ class Minimal(BaseIndicator): def __init__( - self, layer: Layer, feature: Feature, thresholds: tuple = None + self, + layer: Layer, + feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.count = 0 diff --git a/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py b/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py index a9a2b78ba..6eac95f19 100644 --- a/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/poi_density/indicator.py @@ -1,6 +1,7 @@ import logging from io import StringIO from string import Template +from typing import Optional, Tuple import dateutil.parser import matplotlib.pyplot as plt @@ -18,7 +19,10 @@ class PoiDensity(BaseIndicator): def __init__( - self, layer: Layer, feature: Feature, thresholds: tuple = None + self, + layer: Layer, + feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.threshold_yellow = 30 diff --git a/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py b/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py index d53b6fede..4eaac94ba 100644 --- a/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/tags_ratio/indicator.py @@ -1,6 +1,7 @@ import logging from io import StringIO from string import Template +from typing import Optional, Tuple import dateutil.parser import matplotlib.patches as mpatches @@ -14,7 +15,10 @@ class TagsRatio(BaseIndicator): def __init__( - self, layer: Layer, feature: Feature, thresholds: tuple = None + self, + layer: Layer, + feature: Feature, + thresholds: Optional[Tuple[float, float, float, float]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) self.threshold_yellow = 0.75 diff --git a/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml b/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml index f7e88a622..76e6ad5a2 100644 --- a/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml +++ b/workers/ohsome_quality_analyst/indicators/tags_ratio/metadata.yaml @@ -20,4 +20,12 @@ TagsRatio: layer-thresholds: default: [ 0.25, null, 0.75, null] building_count: null + jrc_airport_count: null + jrc_bridge_count: null + jrc_cultural_heritage_site_count: null + jrc_education_count: null + jrc_health_count: null + jrc_mass_gathering_sites_count: null + jrc_power_generation_plant_count: null + jrc_road_length: null major_roads_length: null diff --git a/workers/ohsome_quality_analyst/oqt.py b/workers/ohsome_quality_analyst/oqt.py index b23cb1fe1..7688471d6 100644 --- a/workers/ohsome_quality_analyst/oqt.py +++ b/workers/ohsome_quality_analyst/oqt.py @@ -239,7 +239,7 @@ async def _( logging.info("Layer name: {0:4}".format(layer.name)) indicator_class = name_to_class(class_type="indicator", name=name) - indicator = indicator_class(layer, feature, thresholds=None) + indicator = indicator_class(layer, feature) logging.info("Run preprocessing") await indicator.preprocess() diff --git a/workers/tests/integrationtests/test_api_indicator_geojson_io.py b/workers/tests/integrationtests/test_api_indicator_geojson_io.py index 367ee2258..e7c0e4fb1 100644 --- a/workers/tests/integrationtests/test_api_indicator_geojson_io.py +++ b/workers/tests/integrationtests/test_api_indicator_geojson_io.py @@ -96,7 +96,6 @@ def test_invalid_set_of_arguments(self): "bpolys": bpolys, "dataset": "foo", "featureId": "3", - "thresholds": None, } response = self.client.post(self.endpoint, json=parameters) self.assertEqual(response.status_code, 422) @@ -196,7 +195,6 @@ def test_indicator_layer_data(self): ] }, }, - "thresholds": None, } response = self.client.post(self.endpoint, json=parameters) self.run_tests(response, (self.general_schema, self.feature_schema)) @@ -210,7 +208,6 @@ def test_indicator_layer_data_invalid(self): "description": "", "data": {"result": [{"value": 1.0}]}, # Missing timestamp item }, - "thresholds": None, } response = self.client.post(self.endpoint, json=parameters) self.assertEqual(response.status_code, 422) diff --git a/workers/tests/integrationtests/test_indicator_building_completeness.py b/workers/tests/integrationtests/test_indicator_building_completeness.py index 065640959..34c01c021 100644 --- a/workers/tests/integrationtests/test_indicator_building_completeness.py +++ b/workers/tests/integrationtests/test_indicator_building_completeness.py @@ -24,9 +24,7 @@ def setUp(self): db_client.get_feature_from_db(dataset="regions", feature_id="12") ) self.layer = get_layer_fixture("building_area") - self.indicator = BuildingCompleteness( - feature=self.feature, layer=self.layer, thresholds=None - ) + self.indicator = BuildingCompleteness(feature=self.feature, layer=self.layer) @mock.patch("ohsome_quality_analyst.raster.client.get_config_value") @oqt_vcr.use_cassette() diff --git a/workers/tests/integrationtests/test_indicator_currentness.py b/workers/tests/integrationtests/test_indicator_currentness.py index 614804c82..c7c46afcb 100644 --- a/workers/tests/integrationtests/test_indicator_currentness.py +++ b/workers/tests/integrationtests/test_indicator_currentness.py @@ -26,7 +26,6 @@ def test(self): indicator = Currentness( feature=feature, layer=get_layer_fixture("major_roads_count"), - thresholds=None, ) self.assertIsNotNone(indicator.attribution()) @@ -52,9 +51,7 @@ def test_no_amenities(self): with open(infile, "r") as f: feature = geojson.load(f) - indicator = Currentness( - feature=feature, layer=get_layer_fixture("amenities"), thresholds=None - ) + indicator = Currentness(feature=feature, layer=get_layer_fixture("amenities")) asyncio.run(indicator.preprocess()) self.assertEqual(indicator.element_count, 0) diff --git a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py index 05b4fc8c5..91a317b6f 100644 --- a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py +++ b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_buildings.py @@ -17,9 +17,7 @@ def setUp(self): db_client.get_feature_from_db(dataset="regions", feature_id="3") ) layer = get_layer_fixture("building_count") - self.indicator = GhsPopComparisonBuildings( - feature=feature, layer=layer, thresholds=None - ) + self.indicator = GhsPopComparisonBuildings(feature=feature, layer=layer) @oqt_vcr.use_cassette() def test(self): diff --git a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py index e0ad87565..d3b4adb1f 100644 --- a/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py +++ b/workers/tests/integrationtests/test_indicator_ghs_pop_comparison_roads.py @@ -19,7 +19,6 @@ def setUp(self): self.indicator = GhsPopComparisonRoads( feature=feature, layer=get_layer_fixture("major_roads_length"), - thresholds=None, ) @oqt_vcr.use_cassette() diff --git a/workers/tests/integrationtests/test_indicator_mapping_saturation.py b/workers/tests/integrationtests/test_indicator_mapping_saturation.py index 6086eb30a..6c1fb1294 100644 --- a/workers/tests/integrationtests/test_indicator_mapping_saturation.py +++ b/workers/tests/integrationtests/test_indicator_mapping_saturation.py @@ -19,9 +19,7 @@ def setUp(self): @oqt_vcr.use_cassette() def test(self): # Heidelberg - indicator = MappingSaturation( - layer=self.layer, feature=self.feature, thresholds=None - ) + indicator = MappingSaturation(layer=self.layer, feature=self.feature) self.assertIsNotNone(indicator.attribution()) asyncio.run(indicator.preprocess()) @@ -50,11 +48,7 @@ def test(self): @oqt_vcr.use_cassette() def test_as_feature(self): - indicator = MappingSaturation( - layer=self.layer, - feature=self.feature, - thresholds=None, - ) + indicator = MappingSaturation(layer=self.layer, feature=self.feature) asyncio.run(indicator.preprocess()) indicator.calculate() @@ -97,7 +91,6 @@ def test_immutable_attribute(self): indicator = MappingSaturation( layer=get_layer_fixture("building_count"), feature=feature, - thresholds=None, ) asyncio.run(indicator.preprocess()) indicator.calculate() diff --git a/workers/tests/integrationtests/test_indicator_minimal.py b/workers/tests/integrationtests/test_indicator_minimal.py index 15906ce82..62a9b6848 100644 --- a/workers/tests/integrationtests/test_indicator_minimal.py +++ b/workers/tests/integrationtests/test_indicator_minimal.py @@ -10,7 +10,7 @@ class TestIndicatorMinimal(unittest.TestCase): def setUp(self): feature = get_geojson_fixture("heidelberg-altstadt-feature.geojson") layer = get_layer_fixture("minimal") - self.indicator = Minimal(feature=feature, layer=layer, thresholds=None) + self.indicator = Minimal(feature=feature, layer=layer) @oqt_vcr.use_cassette() def test(self): diff --git a/workers/tests/integrationtests/test_indicator_poi_density.py b/workers/tests/integrationtests/test_indicator_poi_density.py index c5b71805f..72fffb10b 100644 --- a/workers/tests/integrationtests/test_indicator_poi_density.py +++ b/workers/tests/integrationtests/test_indicator_poi_density.py @@ -19,9 +19,7 @@ def setUp(self): ) with open(infile, "r") as f: feature = geojson.load(f) - self.indicator = PoiDensity( - feature=feature, layer=get_layer_fixture("poi"), thresholds=None - ) + self.indicator = PoiDensity(feature=feature, layer=get_layer_fixture("poi")) @oqt_vcr.use_cassette() def test(self): diff --git a/workers/tests/integrationtests/test_indicator_tags_ratio.py b/workers/tests/integrationtests/test_indicator_tags_ratio.py index f9cc69a93..06b875b43 100644 --- a/workers/tests/integrationtests/test_indicator_tags_ratio.py +++ b/workers/tests/integrationtests/test_indicator_tags_ratio.py @@ -23,7 +23,6 @@ def test(self): indicator = TagsRatio( feature=self.feature, layer=get_layer_fixture("jrc_health_count"), - thresholds=None, ) self.assertIsNotNone(indicator.attribution()) diff --git a/workers/tests/integrationtests/test_oqt.py b/workers/tests/integrationtests/test_oqt.py index 6ca02b136..10d57eff3 100644 --- a/workers/tests/integrationtests/test_oqt.py +++ b/workers/tests/integrationtests/test_oqt.py @@ -235,7 +235,6 @@ def test_create_indicator_as_geojson_size_limit_layer_data(self): ] }, }, - thresholds=None, ) asyncio.run(oqt.create_indicator_as_geojson(parameters, size_restriction=True)) From f39dc42f51a43b66a98b6eee7ae312857729ac6e Mon Sep 17 00:00:00 2001 From: Levi Szamek Date: Tue, 14 Feb 2023 16:52:43 +0100 Subject: [PATCH 14/14] fix: use dicts as thresholds for ghspop-indicators --- .../indicators/ghs_pop_comparison_buildings/indicator.py | 6 +++--- .../indicators/ghs_pop_comparison_roads/indicator.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py index 2398179dd..e19c8e65a 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_buildings/indicator.py @@ -23,7 +23,7 @@ def __init__( self, layer: Layer, feature: Feature, - thresholds: Optional[Tuple[float, float, float, float]] = None, + thresholds: Optional[Tuple[dict, dict, dict, dict]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) # Those attributes will be set during lifecycle of the object. @@ -40,13 +40,13 @@ def green_threshold_function(self, pop_per_sqkm) -> float: # TODO: Add docstring # TODO: adjust threshold functions # more precise values? maybe as fraction of the threshold functions? - return 5.0 * np.sqrt(pop_per_sqkm) + return self.thresholds[2]["a"] * np.sqrt(pop_per_sqkm) def yellow_threshold_function(self, pop_per_sqkm) -> float: # TODO: Add docstring # TODO: adjust threshold functions # more precise values? maybe as fraction of the threshold functions? - return 0.75 * np.sqrt(pop_per_sqkm) + return self.thresholds[0]["a"] * np.sqrt(pop_per_sqkm) async def preprocess(self) -> None: raster = get_raster_dataset("GHS_POP_R2019A") diff --git a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py index c9ac45f82..1be3e6d44 100644 --- a/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py +++ b/workers/ohsome_quality_analyst/indicators/ghs_pop_comparison_roads/indicator.py @@ -23,7 +23,7 @@ def __init__( self, layer: Layer, feature: Feature, - thresholds: Optional[Tuple[float, float, float, float]] = None, + thresholds: Optional[Tuple[dict, dict, dict, dict]] = None, ) -> None: super().__init__(layer=layer, feature=feature, thresholds=thresholds) # Those attributes will be set during lifecycle of the object. @@ -39,14 +39,14 @@ def attribution(cls) -> str: def green_threshold_function(self, pop_per_sqkm) -> float: """Return road density threshold for green label.""" if pop_per_sqkm < 5000: - return pop_per_sqkm / 500 + return pop_per_sqkm / self.thresholds[2]["a"] else: return 10 def yellow_threshold_function(self, pop_per_sqkm) -> float: """Return road density threshold for yellow label.""" if pop_per_sqkm < 5000: - return pop_per_sqkm / 1000 + return pop_per_sqkm / self.thresholds[0]["a"] else: return 5