Skip to content

Update energydataservice.py#3687

Closed
Scholdan wants to merge 1 commit intospringfall2008:mainfrom
Scholdan:main
Closed

Update energydataservice.py#3687
Scholdan wants to merge 1 commit intospringfall2008:mainfrom
Scholdan:main

Conversation

@Scholdan
Copy link
Copy Markdown
Contributor

fix(predbat): resolve DST-related minute offset bug in Energi Data Service rates

fix(predbat): resolve DST-related minute offset bug in Energi Data Service rates
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes DST-related minute-offset errors when converting Energi Data Service rate timestamps into Predbat’s per-minute, minutes-from-midnight index format.

Changes:

  • Normalize rate timestamps and midnight_utc to a consistent UTC baseline before computing minute offsets (intended DST fix).
  • Detect the source interval dynamically (15–60 minutes) when expanding slots into per-minute values.
  • Simplify _tariff_for parsing/fallback behavior.

Comment on lines 82 to 85
"""
Convert 15-minute rate data into a per-minute dict keyed by minute offset from midnight_utc.
FIXED: Handles timezone/DST correctly.
"""
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says this converts "15-minute rate data", but the function now dynamically supports 15–60 minute intervals (and Energidataservice is often hourly). Consider updating the docstring/comment to reflect that it expands variable-granularity (15/30/60) slots into per-minute data to avoid confusion for future maintainers.

Copilot uses AI. Check for mistakes.
Comment on lines +131 to +137
# 🔧 SAFETY FIX: ensure minute 0 exists (prevents KeyError)
if rate_data:
if 0 not in rate_data:
min_key = min(rate_data.keys())
shift = min_key
self.log(f"Adjusting rate_data index by {-shift} to align minute 0")
rate_data = {k - shift: v for k, v in rate_data.items()}
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "ensure minute 0 exists" block shifts all keys so the minimum key becomes 0. This changes the semantic meaning of every minute index (minutes-from-midnight) and can silently misalign rates (e.g., if data legitimately starts after midnight due to timezone offsets or partial-day feeds). Instead of shifting, fill missing minute 0 by backfilling/forward-filling from the nearest available interval (or fix the baseline timestamp calculation) while preserving absolute minute offsets.

Suggested change
# 🔧 SAFETY FIX: ensure minute 0 exists (prevents KeyError)
if rate_data:
if 0 not in rate_data:
min_key = min(rate_data.keys())
shift = min_key
self.log(f"Adjusting rate_data index by {-shift} to align minute 0")
rate_data = {k - shift: v for k, v in rate_data.items()}
# 🔧 SAFETY FIX: ensure minute 0 exists (prevents KeyError) without shifting indexes
if rate_data and 0 not in rate_data:
# Back/forward-fill minute 0 from the nearest available minute
nearest_minute = min(rate_data.keys(), key=lambda m: abs(m))
self.log(
f"Filling missing minute 0 rate from minute {nearest_minute}"
)
rate_data[0] = rate_data[nearest_minute]

Copilot uses AI. Check for mistakes.
Comment on lines +91 to +123
# 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

# Detect interval dynamically
if len(data) >= 2:
t0 = self._parse_iso(data[0].get(from_key))
t1 = self._parse_iso(data[1].get(from_key))
if t0 and t1:
interval_minutes = int((t1 - t0).total_seconds() / 60)
if interval_minutes <= 15 or interval_minutes > 60:
interval_minutes = 15
delta = int((t1 - t0).total_seconds() / 60)
if 15 <= delta <= 60:
interval_minutes = delta

for entry in data:
start_time_str = entry.get(from_key)
rate = entry.get(rate_key, 0) * scale

if not use_cent:
# Keep behavior: convert DKK → øre (or cents) if use_cent is False
rate = rate * 100.0

# Parse time robustly
start_time = self._parse_iso(start_time_str)
if start_time is None:
self.log(f"Warn: Invalid time format '{start_time_str}' in data")
continue

# Support naive/aware midnight_utc gracefully
try:
start_minute = int((start_time - midnight_utc).total_seconds() / 60)
except TypeError:
# If midnight_utc is naive, drop tzinfo from start_time for subtraction
start_minute = int((start_time.replace(tzinfo=None) - midnight_utc).total_seconds() / 60)
# 🔧 FIX: normalize timezone properly
if start_time.tzinfo is not None:
start_time = start_time.astimezone(timezone.utc).replace(tzinfo=None)

# Compute minute offset safely
start_minute = int((start_time - midnight).total_seconds() / 60)
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is specifically addressing DST/timezone minute offsets, but the existing Energidataservice unit test only covers a fixed-offset day. Add regression tests for DST start/end days (e.g., Europe/* spring forward and fall back) to validate that minute indices match Predbat’s midnight-based elapsed-minutes convention across the transition and prevent reintroducing the offset bug.

Copilot generated this review using guidance from repository custom instructions.
@springfall2008
Copy link
Copy Markdown
Owner

Fixes put onto a new PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants