fix: classify detailed readings as offpeak/peak on insert - #627
Open
Germwalker wants to merge 1 commit into
Open
fix: classify detailed readings as offpeak/peak on insert#627Germwalker wants to merge 1 commit into
Germwalker wants to merge 1 commit into
Conversation
Detail.parse_datas() computed measure_type for every detailed reading (consumption/production, 30-min granularity) with is_between() against self.offpeak_hours, but the whole computation was commented out and insert_detail() was called with measure_type="" hard-coded. Both is_between() (below in the same file) and self.offpeak_hours (built in __init__) are otherwise unused, so nothing else depends on the current disabled state. Consequence: consumption_detail.measure_type (and production_detail.measure_type) is always "", so Stat.get_mesure_type(), which reads this column, cannot split any detailed reading into offpeak/peak, and every offpeak/peak simulation based on detailed data reports 0 EUR for the offpeak share regardless of the contract's offpeak hours. Nothing in the history or comments explains why this block was disabled - it is already commented out in the 0.13.4 release, with no accompanying note. Was this intentionally disabled, e.g. because of a known correctness issue with is_between() or performance concerns on large imports? If so this PR should be closed; if not, re-enabling the existing code look like the fix. Fix: restore the measure_type computation and pass the real value to insert_detail() instead of the hard-coded "".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Symptom
consumption_detail.measure_type(andproduction_detail.measure_type) is always an emptystring, for every row, on a stock 0.13.4 install. Any statistic or export based on detailed
(30-minute) readings that needs to split offpeak from peak reports 0 for the offpeak side.
Root cause
In
Detail.parse_datas()(src/models/query_detail.py, around lines 126-144), the wholeblock that computes
measure_typefromself.offpeak_hoursandis_between()is commentedout, and a few lines below,
insert_detail()is called withmeasure_type=""hard-coded.Both dependencies the commented block needs are present and otherwise unused in this file:
self.offpeak_hoursis built in__init__(lines ~53-60) andis_between()is defined belowin the same module (~line 276). Nothing else in the file relies on the current disabled
state.
Open question — was this intentional?
Nothing in the code, comments, or commit history explains why this block is disabled — it
is already commented out in the 0.13.4 release, with no accompanying note. Before this is
merged: was this intentionally disabled, e.g. because of a known correctness issue with
is_between(), a performance concern on large imports, or something else not visible from thediff alone? If so, this PR should be closed rather than merged. If it was simply dead code
that nobody got around to removing or re-enabling, then restoring it looks like the right fix
— happy to adjust based on your answer.
Fix
Restore the
measure_typecomputation exactly as written (only reformatted to match theproject's quote style), and pass the computed value to
insert_detail()instead of thehard-coded
"".Proof (measured on real data)
Same 2.7-year dataset as the companion PR (offpeak hours propagation): 35272 rows in
consumption_detail, all withmeasure_type=""before this fix. After enabling both this fixand the contract offpeak-hours propagation together, 35112 rows reclassify to
HC/HPcorrectly, and the offpeak/peak split (2025, one 15 kVA point) goes from
HC=0 EUR/HP=1396.17 EURtoHC=281.02 EUR/HP=1028.43 EUR.Scope
One file,
src/models/query_detail.py, 19 lines touched (all inside the existing block).No other file touched.