feat: support reading tilt and orientation from Data Platform metadat…

forecast-inference/forecast_inference/data/pv_data_sources.py

@@ -59,8 +59,9 @@ class DbPvDataSource(PvDataSource):
     When the `READ_FROM_DATA_PLATFORM` environment variable is set to "true", generation
     power values and location metadata (latitude, longitude, capacity_kw) are instead
     read from the Data Platform, matched to each site via its `client_location_name`.
-    `tilt` and `orientation` have no Data Platform equivalent, so they always come from
-    the database.
+    `tilt` and `orientation` have no native Data Platform field; when available they're
+    read from the location's `metadata` Struct (see `fetch_location_for_one_site_from_dp`),
+    falling back to the database for sites not yet backfilled with that metadata.
     """
 
     def __init__(
@@ -171,7 +172,7 @@ def get(
 
         # Add the metadata associated with the PV systems. Prefer values fetched from the Data
         # Platform (when reading from there), falling back to the database for sites the DP
-        # doesn't know about, and for the fields (tilt, orientation) it doesn't hold at all.
+        # doesn't know about, and for tilt/orientation when not yet present in DP metadata.
         meta = {
             str(site.location_uuid): {
                 key: dp_locations.get(str(site.location_uuid), {}).get(

forecast-inference/forecast_inference/data_platform/load.py

@@ -87,11 +87,25 @@ async def fetch_generation_for_one_site_from_dp(
     return data
 
 
+def _get_metadata_float(summary: LocationSummary, key: str) -> float | None:
+    """Read a numeric value out of a DP location's generic `metadata` Struct, if present."""
+    value = summary.metadata.fields.get(key)
+    if value is None:
+        return None
+    return value.number_value
+
+
 def fetch_location_for_one_site_from_dp(
     loc_map: dict[str, LocationSummary],
     site: LocationSQL,
 ) -> dict | None:
-    """Look up a site's location metadata (latitude, longitude, capacity_kw) in the DP."""
+    """Look up a site's location metadata (latitude, longitude, capacity_kw) in the DP.
+
+    `tilt`/`orientation` have no native DP field, so they're read from the location's
+    `metadata` Struct when present (see `pv-site-api`'s `create_location`/`update_location`,
+    which write them there). When absent from `metadata` (e.g. not yet backfilled), the key
+    is omitted here so the caller falls back to the database value instead of a bare `None`.
+    """
     if not site.client_location_name:
         return None
 
@@ -100,12 +114,22 @@ def fetch_location_for_one_site_from_dp(
     if not summary:
         return None
 
-    return {
+    location = {
         "latitude": summary.latlng.latitude,
         "longitude": summary.latlng.longitude,
         "capacity_kw": summary.effective_capacity_watts / 1000.0,
     }
 
+    tilt = _get_metadata_float(summary, "tilt")
+    if tilt is not None:
+        location["tilt"] = tilt
+
+    orientation = _get_metadata_float(summary, "orientation")
+    if orientation is not None:
+        location["orientation"] = orientation
+
+    return location
+
 
 async def get_generation_from_dp(
     client: DataPlatformClient,

forecast-inference/tests/unit/data/test_pv_data_sources.py

@@ -103,15 +103,21 @@ def test_fetch_generation_for_one_site_from_dp_site_not_in_dp(self, dp_site):
         mock_client.get_observations_as_timeseries.assert_not_called()
 
 
+def _mock_summary(latitude=52.5, longitude=-2.5, capacity_watts=5000, metadata_fields=None):
+    """Build a MagicMock DP `LocationSummary`, with a real (non-auto-mocking) metadata dict."""
+    mock_summary = MagicMock()
+    mock_summary.latlng.latitude = latitude
+    mock_summary.latlng.longitude = longitude
+    mock_summary.effective_capacity_watts = capacity_watts
+    mock_summary.metadata.fields = metadata_fields or {}
+    return mock_summary
+
+
 class TestFetchLocationFromDp:
     """Test the module-level `fetch_location_for_one_site_from_dp` helper."""
 
     def test_returns_latitude_longitude_and_capacity(self, dp_site):
-        mock_summary = MagicMock()
-        mock_summary.latlng.latitude = 52.5
-        mock_summary.latlng.longitude = -2.5
-        mock_summary.effective_capacity_watts = 5000
-        loc_map = {_sanitize(dp_site_name): mock_summary}
+        loc_map = {_sanitize(dp_site_name): _mock_summary()}
 
         location = fetch_location_for_one_site_from_dp(loc_map, dp_site)
 
@@ -122,14 +128,48 @@ def test_returns_latitude_longitude_and_capacity(self, dp_site):
         }
 
     def test_site_not_in_dp_returns_none(self, dp_site):
-        location = fetch_location_for_one_site_from_dp({"some-other-site": MagicMock()}, dp_site)
+        location = fetch_location_for_one_site_from_dp(
+            {"some-other-site": _mock_summary()}, dp_site
+        )
         assert location is None
 
     def test_site_without_client_location_name_returns_none(self, dp_site):
         dp_site.client_location_name = None
-        location = fetch_location_for_one_site_from_dp({_sanitize(dp_site_name): MagicMock()}, dp_site)
+        location = fetch_location_for_one_site_from_dp(
+            {_sanitize(dp_site_name): _mock_summary()}, dp_site
+        )
         assert location is None
 
+    def test_includes_tilt_and_orientation_when_present_in_metadata(self, dp_site):
+        tilt_value = MagicMock()
+        tilt_value.number_value = 35.0
+        orientation_value = MagicMock()
+        orientation_value.number_value = 170.0
+        loc_map = {
+            _sanitize(dp_site_name): _mock_summary(
+                metadata_fields={"tilt": tilt_value, "orientation": orientation_value}
+            )
+        }
+
+        location = fetch_location_for_one_site_from_dp(loc_map, dp_site)
+
+        assert location == {
+            "latitude": 52.5,
+            "longitude": -2.5,
+            "capacity_kw": 5.0,
+            "tilt": 35.0,
+            "orientation": 170.0,
+        }
+
+    def test_omits_tilt_and_orientation_when_absent_from_metadata(self, dp_site):
+        """No `tilt`/`orientation` keys in DP metadata → caller falls back to the database."""
+        loc_map = {_sanitize(dp_site_name): _mock_summary(metadata_fields={})}
+
+        location = fetch_location_for_one_site_from_dp(loc_map, dp_site)
+
+        assert "tilt" not in location
+        assert "orientation" not in location
+
 
 class TestDbPvDataSourceReadsFromDataPlatform:
     """Test `DbPvDataSource.get()` with `READ_FROM_DATA_PLATFORM=true`."""
@@ -147,6 +187,7 @@ def test_reads_generation_and_location_metadata_from_dp(
         mock_summary.effective_capacity_watts = 5000  # 5 kW, different from DB's 4 kW
         mock_summary.latlng.latitude = 52.5  # different from DB's 51.0
         mock_summary.latlng.longitude = -2.5  # different from DB's -1.0
+        mock_summary.metadata.fields = {}  # no tilt/orientation in DP metadata for this site
 
         mock_val = MagicMock()
         mock_val.timestamp_utc = dt.datetime(2024, 6, 1, 10, 0, tzinfo=dt.UTC)
@@ -182,10 +223,56 @@ def test_reads_generation_and_location_metadata_from_dp(
         assert result["longitude"].sel(id=site_uuid).item() == pytest.approx(-2.5)
         assert result["capacity"].sel(id=site_uuid).item() == pytest.approx(5.0)
 
-        # tilt/orientation have no DP equivalent, so they still come from the DB.
+        # tilt/orientation have no DP equivalent and weren't present in metadata here,
+        # so they still come from the DB.
         assert result["tilt"].sel(id=site_uuid).item() == pytest.approx(30.0)
         assert result["orientation"].sel(id=site_uuid).item() == pytest.approx(180.0)
 
+    def test_reads_tilt_and_orientation_from_dp_metadata_when_present(
+        self, monkeypatch, database_connection, dp_site
+    ):
+        """When DP location metadata carries tilt/orientation, prefer those over the DB."""
+        monkeypatch.setenv("READ_FROM_DATA_PLATFORM", "true")
+        site_uuid = str(dp_site.location_uuid)
+
+        mock_client = AsyncMock()
+
+        tilt_value = MagicMock()
+        tilt_value.number_value = 35.0  # different from DB's 30.0
+        orientation_value = MagicMock()
+        orientation_value.number_value = 170.0  # different from DB's 180.0
+
+        mock_summary = MagicMock()
+        mock_summary.location_uuid = "dp-uuid-123"
+        mock_summary.effective_capacity_watts = 4000
+        mock_summary.latlng.latitude = 51.0
+        mock_summary.latlng.longitude = -1.0
+        mock_summary.metadata.fields = {"tilt": tilt_value, "orientation": orientation_value}
+
+        mock_obs_resp = MagicMock()
+        mock_obs_resp.values = []
+        mock_client.get_observations_as_timeseries.return_value = mock_obs_resp
+
+        with (
+            patch(
+                "forecast_inference.data_platform.load.get_dataplatform_client",
+                return_value=_mock_dp_context(mock_client),
+            ),
+            patch(
+                "forecast_inference.data_platform.load.fetch_dp_location_map",
+                new=AsyncMock(return_value={_sanitize(dp_site_name): mock_summary}),
+            ),
+        ):
+            pv_data_source = DbPvDataSource(database_connection)
+            result = pv_data_source.get(
+                [site_uuid],
+                start_ts=dt.datetime(2024, 6, 1, tzinfo=dt.UTC),
+                end_ts=dt.datetime(2024, 6, 2, tzinfo=dt.UTC),
+            )
+
+        assert result["tilt"].sel(id=site_uuid).item() == pytest.approx(35.0)
+        assert result["orientation"].sel(id=site_uuid).item() == pytest.approx(170.0)
+
     def test_site_not_found_in_dp_falls_back_to_db_metadata(
         self, monkeypatch, database_connection, dp_site
     ):
@@ -254,6 +341,7 @@ def test_works_when_called_from_within_a_running_event_loop(
         mock_summary.effective_capacity_watts = 4000
         mock_summary.latlng.latitude = 51.0
         mock_summary.latlng.longitude = -1.0
+        mock_summary.metadata.fields = {}
 
         async def _call_get_from_within_a_running_loop():
             pv_data_source = DbPvDataSource(database_connection)
@@ -297,6 +385,7 @@ def test_location_map_is_cached_across_calls(self, monkeypatch, database_connect
         mock_summary.effective_capacity_watts = 4000
         mock_summary.latlng.latitude = 51.0
         mock_summary.latlng.longitude = -1.0
+        mock_summary.metadata.fields = {}
         mock_fetch_loc_map = AsyncMock(return_value={_sanitize(dp_site_name): mock_summary})
 
         with (