Skip to content

Fix get_all_periods for RepresentativePeriods and OperationalScenarios - #57

Merged
JulStraus merged 2 commits into
mainfrom
fix/read_rp_osc_csvs
Oct 7, 2026
Merged

JulStraus merged 2 commits into
mainfrom
fix/read_rp_osc_csvs

Conversation

@JulStraus

Copy link
Copy Markdown
Member

Problem

get_all_periods! called itself on the raw inner time structures (ts.operational, ts.rep_periods, ts.scenarios). This meant:

  • Nested periods lost the strategic, representative or scenario indices of the levels above them.
  • Operational periods were only collected for TwoLevel. For a top-level RepresentativePeriods or OperationalScenarios, they were missing from the period mapping.

Solution

The function now takes two arguments: the current period and the time structure it belongs to.

It walks down through the period objects (strategic_periods, repr_periods, opscenarios), and the time structure argument decides which method runs. At the SimpleTimes level it collects the operational periods, so each one keeps its full index chain. New tests in test/test_utils.jl cover all combinations of TwoLevel, RepresentativePeriods, OperationalScenarios and SimpleTimes.

@JulStraus
JulStraus requested review from Zetison and dqpinel October 6, 2026 12:01
@JulStraus JulStraus added the bug Something isn't working label Oct 6, 2026
@JulStraus

JulStraus commented Oct 6, 2026 •

Copy link
Copy Markdown
Member Author

One think I forgot to mention in the description above: I did not increase the version number, as I have a pretty much finished PR for #55 which will be registered together with this PR.

Comment thread src/utils_GUI/GUI_utils.jl
Comment thread src/utils_GUI/GUI_utils.jl Outdated
@JulStraus
JulStraus merged commit f49d014 into main Oct 7, 2026
3 checks passed
@JulStraus
JulStraus deleted the fix/read_rp_osc_csvs branch October 7, 2026 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants