Energydataservice timezone aware changes#3699
Conversation
fix(predbat): resolve DST-related minute offset bug in Energi Data Service rates
There was a problem hiding this comment.
Pull request overview
Addresses DST-related minute-offset misalignment when expanding Energi Data Service rate timestamps into PredBat’s minute-indexed rate dictionaries, and adds regression tests to prevent future DST/offset-shift regressions.
Changes:
- Update Energi Data Service rate expansion to compute minute offsets using UTC-normalized datetimes and to fill missing leading minutes without shifting the whole dataset.
- Add DST regression tests covering spring-forward, fall-back, and “missing minute 0” fill-forward behavior.
- Update the unit test runner wiring to execute the expanded Energi Data Service test suite.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/predbat/energydataservice.py | Normalizes timestamps to UTC for minute offset computation and fills missing initial minutes without shifting rate keys. |
| apps/predbat/tests/test_energydataservice.py | Adds DST and fill-forward regression tests; introduces a run_energydataservice_tests wrapper. |
| apps/predbat/unit_test.py | Switches the test registry entry to run the new Energi Data Service test suite wrapper. |
| # Normalize midnight to naive (local comparison baseline) | ||
| if midnight_utc.tzinfo is not None: | ||
| midnight = midnight_utc.astimezone(timezone.utc).replace(tzinfo=None) | ||
| else: | ||
| midnight = midnight_utc |
There was a problem hiding this comment.
This comment is inaccurate: the code normalizes midnight_utc to a naive UTC datetime, not a “local comparison baseline”. Please adjust the comment (and/or rename the local variable) so it clearly reflects the UTC normalization to prevent confusion when reasoning about DST behavior.
| Convert 15-minute rate data into a per-minute dict keyed by minute offset from midnight_utc. | ||
| FIXED: Handles timezone/DST correctly. |
There was a problem hiding this comment.
The docstring says this converts “15-minute rate data”, but the implementation detects the interval dynamically (15–60 min) and is also used for hourly data. Please update the docstring to describe interval-agnostic expansion to per-minute slots to avoid misleading future readers.
| Convert 15-minute rate data into a per-minute dict keyed by minute offset from midnight_utc. | |
| FIXED: Handles timezone/DST correctly. | |
| Convert fixed-interval rate data into a per-minute dict keyed by minute offset from midnight_utc. | |
| The interval (typically 15–60 minutes) is detected dynamically from consecutive timestamps. | |
| Handles timezone/DST correctly. |
#3687 with fixes