diff --git a/forecast-inference/forecast_inference/data/pv_data_sources.py b/forecast-inference/forecast_inference/data/pv_data_sources.py index d5a2973..4c3f108 100644 --- a/forecast-inference/forecast_inference/data/pv_data_sources.py +++ b/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( diff --git a/forecast-inference/forecast_inference/data_platform/load.py b/forecast-inference/forecast_inference/data_platform/load.py index 3a4acc9..3f6b0cb 100644 --- a/forecast-inference/forecast_inference/data_platform/load.py +++ b/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, diff --git a/forecast-inference/tests/unit/data/test_pv_data_sources.py b/forecast-inference/tests/unit/data/test_pv_data_sources.py index ade7fdb..a721eda 100644 --- a/forecast-inference/tests/unit/data/test_pv_data_sources.py +++ b/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 (