fix incorrect generation of related coverage identifiers (#180)

* Adding integration test case for coverage time series endpoint * Fixing incorrect determination of related coverages
geobeyond · Jul 25, 2024 · aecb52f · aecb52f
1 parent 3738a64
commit aecb52f
Show file tree

Hide file tree

Showing 9 changed files with 3,794 additions and 259 deletions.
diff --git a/arpav_ppcv/database.py b/arpav_ppcv/database.py
@@ -777,6 +777,19 @@ def collect_all_configuration_parameters(
     return result
 
 
+def create_configuration_parameter_value(
+    session: sqlmodel.Session,
+    parameter_value: coverages.ConfigurationParameterValueCreate,
+) -> coverages.ConfigurationParameterValue:
+    db_param_value = coverages.ConfigurationParameterValue(
+        **parameter_value.model_dump()
+    )
+    session.add(db_param_value)
+    session.commit()
+    session.refresh(db_param_value)
+    return db_param_value
+
+
 def create_configuration_parameter(
     session: sqlmodel.Session,
     configuration_parameter_create: coverages.ConfigurationParameterCreate,
@@ -1307,6 +1320,68 @@ def collect_all_coverage_identifiers(
     return cov_ids
 
 
+def ensure_uncertainty_type_configuration_parameters_exist(
+    session: sqlmodel.Session,
+) -> tuple[
+    coverages.ConfigurationParameterValue, coverages.ConfigurationParameterValue
+]:
+    """Ensure that the `uncertainty_type` configuration parameter exists.
+
+    Because internally we make use of the `uncertainty_type` configuration parameter,
+    we must ensure it and its respective values exist. This can happen if an admin
+    user deletes them by accident.
+    """
+    param_name = "uncertainty_type"
+    lower_bound_name = "lower_bound"
+    upper_bound_name = "upper_bound"
+    param = get_configuration_parameter_by_name(session, param_name)
+    if param is None:
+        create_configuration_parameter(
+            session,
+            coverages.ConfigurationParameterCreate(
+                name=param_name,
+                allowed_values=[
+                    coverages.ConfigurationParameterValueCreateEmbeddedInConfigurationParameter(
+                        name=lower_bound_name,
+                    ),
+                    coverages.ConfigurationParameterValueCreateEmbeddedInConfigurationParameter(
+                        name=upper_bound_name,
+                    ),
+                ],
+            ),
+        )
+        lower_bound_value = get_configuration_parameter_value_by_names(
+            session, param_name, lower_bound_name
+        )
+        upper_bound_value = get_configuration_parameter_value_by_names(
+            session, param_name, upper_bound_name
+        )
+    else:
+        lower_bound_value = get_configuration_parameter_value_by_names(
+            session, param_name, lower_bound_name
+        )
+        upper_bound_value = get_configuration_parameter_value_by_names(
+            session, param_name, upper_bound_name
+        )
+        if lower_bound_value is None:
+            lower_bound_value = create_configuration_parameter_value(
+                session,
+                parameter_value=coverages.ConfigurationParameterValueCreate(
+                    name="lower_bound",
+                    configuration_parameter_id=param.id,
+                ),
+            )
+        if upper_bound_value is None:
+            upper_bound_value = create_configuration_parameter_value(
+                session,
+                parameter_value=coverages.ConfigurationParameterValueCreate(
+                    name="upper_bound",
+                    configuration_parameter_id=param.id,
+                ),
+            )
+    return lower_bound_value, upper_bound_value
+
+
 def _get_total_num_records(session: sqlmodel.Session, statement):
     return session.exec(
         sqlmodel.select(sqlmodel.func.count()).select_from(statement)

diff --git a/arpav_ppcv/operations.py b/arpav_ppcv/operations.py
@@ -38,6 +38,7 @@
 
 def get_climate_barometer_time_series(
     settings: ArpavPpcvSettings,
+    session: sqlmodel.Session,
     coverage: coverages.CoverageInternal,
     smoothing_strategies: list[base.CoverageDataSmoothingStrategy] = [  # noqa
         base.CoverageDataSmoothingStrategy.NO_SMOOTHING
@@ -50,7 +51,9 @@ def get_climate_barometer_time_series(
     df = _get_climate_barometer_data(settings, coverage)
     dfs.append((coverage, df))
     if include_uncertainty:
-        lower_cov, upper_cov = get_related_uncertainty_coverage_configurations(coverage)
+        lower_cov, upper_cov = get_related_uncertainty_coverage_configurations(
+            session, coverage
+        )
         if lower_cov is not None:
             lower_df = _get_climate_barometer_data(settings, lower_cov)
             dfs.append((lower_cov, lower_df))
@@ -478,16 +481,21 @@ def process_station_data_smoothing_strategy(
 
 
 def get_related_uncertainty_coverage_configurations(
+    session: sqlmodel.Session,
     coverage: coverages.CoverageInternal,
 ) -> tuple[coverages.CoverageInternal | None, coverages.CoverageInternal | None]:
-    used_values = coverage.configuration.retrieve_used_values(coverage.identifier)
+    used_values = [
+        pv.configuration_parameter_value
+        for pv in coverage.configuration.retrieve_used_values(coverage.identifier)
+    ]
+    lower_, upper_ = database.ensure_uncertainty_type_configuration_parameters_exist(
+        session
+    )
     if (
         lower_cov_conf
         := coverage.configuration.uncertainty_lower_bounds_coverage_configuration
     ):
-        lower_cov_id = lower_cov_conf.build_coverage_identifier(
-            [pv.configuration_parameter_value for pv in used_values]
-        )
+        lower_cov_id = lower_cov_conf.build_coverage_identifier(used_values + [lower_])
         lower_cov = coverages.CoverageInternal(
             configuration=lower_cov_conf, identifier=lower_cov_id
         )
@@ -497,9 +505,7 @@ def get_related_uncertainty_coverage_configurations(
         upper_cov_conf
         := coverage.configuration.uncertainty_upper_bounds_coverage_configuration
     ):
-        upper_cov_id = upper_cov_conf.build_coverage_identifier(
-            [pv.configuration_parameter_value for pv in used_values]
-        )
+        upper_cov_id = upper_cov_conf.build_coverage_identifier(used_values + [upper_])
         upper_cov = coverages.CoverageInternal(
             configuration=upper_cov_conf, identifier=upper_cov_id
         )
@@ -515,9 +521,27 @@ def get_related_coverages(
     used_values = coverage.configuration.retrieve_used_values(coverage.identifier)
     for related_ in coverage.configuration.secondary_coverage_configurations:
         related_cov_conf = related_.secondary_coverage_configuration
-        related_id = related_cov_conf.build_coverage_identifier(
-            [pv.configuration_parameter_value for pv in used_values]
-        )
+        possible_used = [
+            pv.configuration_parameter_value for pv in related_cov_conf.possible_values
+        ]
+        values_to_use = []
+        for used_value in used_values:
+            if used_value in possible_used:
+                values_to_use.append(used_value)
+            else:
+                used_param_id = (
+                    used_value.configuration_parameter_value.configuration_parameter_id
+                )
+                try:
+                    possible = [
+                        cp
+                        for cp in possible_used
+                        if cp.configuration_parameter_id == used_param_id
+                    ][0]
+                    values_to_use.append(possible)
+                except IndexError:
+                    logger.warning(f"Could not find a usable value for {used_value}")
+        related_id = related_cov_conf.build_coverage_identifier(values_to_use)
         related_covs.append(
             coverages.CoverageInternal(
                 configuration=related_cov_conf, identifier=related_id
@@ -553,7 +577,9 @@ def get_coverage_time_series(
     start, end = _parse_temporal_range(temporal_range)
     to_retrieve_from_ncss = [coverage]
     if include_coverage_uncertainty:
-        lower_cov, upper_cov = get_related_uncertainty_coverage_configurations(coverage)
+        lower_cov, upper_cov = get_related_uncertainty_coverage_configurations(
+            session, coverage
+        )
         if lower_cov is not None:
             to_retrieve_from_ncss.append(lower_cov)
         if upper_cov is not None:

diff --git a/arpav_ppcv/schemas/coverages.py b/arpav_ppcv/schemas/coverages.py
@@ -57,6 +57,15 @@ class ConfigurationParameterValue(sqlmodel.SQLModel, table=True):
     )
 
 
+class ConfigurationParameterValueCreate(sqlmodel.SQLModel):
+    name: str
+    configuration_parameter_id: uuid.UUID
+    display_name_english: Optional[str] = None
+    display_name_italian: Optional[str] = None
+    description_english: Optional[str] = None
+    description_italian: Optional[str] = None
+
+
 class ConfigurationParameter(sqlmodel.SQLModel, table=True):
     id: uuid.UUID = sqlmodel.Field(default_factory=uuid.uuid4, primary_key=True)
     name: str = sqlmodel.Field(unique=True, index=True)

diff --git a/arpav_ppcv/webapp/api_v2/routers/coverages.py b/arpav_ppcv/webapp/api_v2/routers/coverages.py
@@ -417,6 +417,7 @@ def get_climate_barometer_time_series(
             try:
                 time_series = operations.get_climate_barometer_time_series(
                     settings,
+                    db_session,
                     coverage,
                     smoothing_strategies=data_smoothing,
                     include_uncertainty=include_uncertainty,

diff --git a/pyproject.toml b/pyproject.toml
@@ -73,8 +73,8 @@ pytest-cov = "^4.1.0"
 pytest-django = "^4.8.0"
 dagger-io = "^0.9.10"
 ruff = "^0.2.2"
-pytest-httpx = "^0.30.0"
 pre-commit = "^3.7.1"
+pytest-httpx = "^0.30.0"
 
 
 [tool.poetry.group.jupyter]

diff --git a/tests/conftest.py b/tests/conftest.py
@@ -253,6 +253,55 @@ def sample_coverage_configurations(
     return db_cov_confs
 
 
+@pytest.fixture()
+def sample_tas_csv_data():
+    return """
+    time,station,latitude[unit="degrees_north"],longitude[unit="degrees_east"],tas[unit="degC"]
+    1976-02-15T12:00:00Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,2.640222
+    1977-02-14T17:57:04.390Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.131799
+    1978-02-14T23:54:08.780Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,2.9139953
+    1979-02-15T05:51:13.171Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.9587035
+    1980-02-15T11:48:17.561Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.5937133
+    1981-02-14T17:45:21.951Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.7524657
+    1982-02-14T23:42:26.341Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.8758483
+    1983-02-15T05:39:30.732Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.5044188
+    1984-02-15T11:36:35.122Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,2.284906
+    1985-02-14T17:33:39.512Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.2877746
+    1986-02-14T23:30:43.902Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.3630004
+    1987-02-15T05:27:48.293Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,2.611383
+    1988-02-15T11:24:52.683Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.5216613
+    1989-02-14T17:21:57.073Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.7202392
+    1990-02-14T23:19:01.463Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.1510253
+    1991-02-15T05:16:05.854Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.5604796
+    1992-02-15T11:13:10.244Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,2.830011
+    1993-02-14T17:10:14.634Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.3071227
+    1994-02-14T23:07:19.024Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.4500365
+    1995-02-15T05:04:23.415Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.8746276
+    1996-02-15T11:01:27.805Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.0703063
+    1997-02-14T16:58:32.195Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.0519347
+    1998-02-14T22:55:36.585Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.9186034
+    1999-02-15T04:52:40.976Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.3369384
+    2000-02-15T10:49:45.366Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.413568
+    2001-02-14T16:46:49.756Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.7551513
+    2002-02-14T22:43:54.146Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.6977477
+    2003-02-15T04:40:58.537Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.3922668
+    2004-02-15T10:38:02.927Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.298364
+    2005-02-14T16:35:07.317Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.7203918
+    2006-02-14T22:32:11.707Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,5.3815246
+    2007-02-15T04:29:16.098Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.568109
+    2008-02-15T10:26:20.488Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.08172
+    2009-02-14T16:23:24.878Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.7300353
+    2010-02-14T22:20:29.268Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.7169127
+    2011-02-15T04:17:33.659Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.357843
+    2012-02-15T10:14:38.049Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,2.469293
+    2013-02-14T16:11:42.439Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.4914489
+    2014-02-14T22:08:46.829Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.1174865
+    2015-02-15T04:05:51.220Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,4.338098
+    2016-02-15T10:02:55.610Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,5.111444
+    2017-02-14T16:00:00Z,GridPointRequestedAt[44.952N_11.547E],44.953,11.547,3.911859
+    """.strip()
+
+
 def _override_get_settings():
     standard_settings = config.get_settings()
     return standard_settings