Skip to content

forecast/all get timeseries if gsp_ids are set#196

Merged
peterdudfield merged 3 commits intomainfrom
issue/forecast/all
Mar 4, 2026
Merged

forecast/all get timeseries if gsp_ids are set#196
peterdudfield merged 3 commits intomainfrom
issue/forecast/all

Conversation

@peterdudfield
Copy link
Contributor

@peterdudfield peterdudfield commented Mar 3, 2026

Pull Request

Description

For the forecast/all route, if gsp_ids is not None, then we get full predictions.
If gsp_ids is None, then we just get one timestamp

We decided not to limit to gsp ids of 10, as the DNO view on the UI loads more than 10 gsp ids

#195

How Has This Been Tested?

  • CI tests
  • added a extra text
  • ran locally

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@peterdudfield peterdudfield changed the title TDD: add test for gsp_ids, for forecast/all forecast/all get timeseries if gsp_ids are set Mar 3, 2026
@peterdudfield peterdudfield requested a review from devsjc March 3, 2026 20:20
)
# if gsp ids are not set, then we use snapshot method, which gets all gsps for one timestamp
# if gsp_ids is set, then we loop over all gsp ids to get forecasts. The UI current needs this.
if gsp_ids is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Wasn't there an intention to make it so fewer than 10 could have simultaneous timeseries collection or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This changed, as we realised that DNOs have a lot more than 10 gsps in them, and we need it to work for that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea would be to move the UI over to use gsp/{gps_id}/forecast when possible. But atleast this should work

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh do we get timeseries forecasts for whole DNOs at a time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yea DNO is done by gsp/forecast/all/?gsp_ids=[x_1,x2, ...x_20,....]
and we can select mutlie DNOs too

@peterdudfield peterdudfield merged commit b95e456 into main Mar 4, 2026
8 checks passed
@peterdudfield peterdudfield deleted the issue/forecast/all branch March 4, 2026 10:55
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.

2 participants