Skip to content

fix(api): Do not error for empty timeseries#160

Merged
devsjc merged 1 commit intomainfrom
devsjc/timeseries-no-values
Apr 28, 2026
Merged

fix(api): Do not error for empty timeseries#160
devsjc merged 1 commit intomainfrom
devsjc/timeseries-no-values

Conversation

@devsjc
Copy link
Copy Markdown
Contributor

@devsjc devsjc commented Apr 28, 2026

Contribution Checklist

  • Have you followed the Open Climate Fix Contribution Guidelines?
  • Have you referenced the Issue this PR addresses, where applicable?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added a summary of the changes?
  • Have you written new tests for your changes, where applicable?
  • Have you successfully run make lint with your changes locally?
  • Have you successfully run make test with your changes locally?

Warning

PRs may be closed if all the above boxes are not checked.

Changes in this Pull Request

In the GetForecastAsTimeseries route, instead of returning an error when no values are returned as has been done previously, now this is accepted as a valid response, and simply an empty list is returned instead.

In the GetForecastAsTimeseries route, instead of returning an error when
no values are returned as has been done previously, now this is accepted
as a valid response, and simply an empty list is returned instead.
Copy link
Copy Markdown
Contributor

@peterdudfield peterdudfield left a comment

Choose a reason for hiding this comment

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

Looks good (ive approved before seeing if the tests pass - so wait for them)

@github-actions
Copy link
Copy Markdown

Benchmark Results

Benchmark results
?   	github.com/openclimatefix/data-platform/cmd	[no test files]
?   	github.com/openclimatefix/data-platform/internal/gen/ocf/dp	[no test files]
?   	github.com/openclimatefix/data-platform/internal/interceptors	[no test files]
PASS
ok  	github.com/openclimatefix/data-platform/internal/server/dummy	0.004s
{"level":"debug","time":"2026-04-28T13:44:18Z","message":"Completed migrations"}
goos: linux
goarch: amd64
pkg: github.com/openclimatefix/data-platform/internal/server/postgres
cpu: AMD EPYC 9V74 80-Core Processor                
BenchmarkPostgresClient/small/GetForecastAsTimeseries-4         	      74	  15132524 ns/op
BenchmarkPostgresClient/small/GetForecastAtTimestamp-4          	     368	   3186967 ns/op
BenchmarkPostgresClient/small/GetObservationsAsTimeseries-4     	    1300	    882312 ns/op
BenchmarkPostgresClient/small/CreateForecast-4                  	     128	   9082166 ns/op
PASS
ok  	github.com/openclimatefix/data-platform/internal/server/postgres	75.657s
?   	github.com/openclimatefix/data-platform/internal/server/postgres/gen	[no test files]
Benchmark vs base branch
goos: linux
goarch: amd64
pkg: github.com/openclimatefix/data-platform/internal/server/postgres
cpu: AMD EPYC 9V74 80-Core Processor                
                                                   │ bench-main.txt │ bench-devsjc-timeseries-no-values.txt │
                                                   │     sec/op     │    sec/op     vs base                 │
PostgresClient/small/GetForecastAsTimeseries-4         18.98m ± ∞ ¹   15.13m ± ∞ ¹        ~ (p=1.000 n=1) ²
PostgresClient/small/GetForecastAtTimestamp-4          4.260m ± ∞ ¹   3.187m ± ∞ ¹        ~ (p=1.000 n=1) ²
PostgresClient/small/GetObservationsAsTimeseries-4    1187.7µ ± ∞ ¹   882.3µ ± ∞ ¹        ~ (p=1.000 n=1) ²
PostgresClient/small/CreateForecast-4                 11.701m ± ∞ ¹   9.082m ± ∞ ¹        ~ (p=1.000 n=1) ²
geomean                                                5.790m         4.434m        -23.42%
¹ need >= 6 samples for confidence interval at level 0.95
² need >= 4 samples to detect a difference at alpha level 0.05

@devsjc devsjc merged commit ed3c82e into main Apr 28, 2026
4 checks passed
@devsjc devsjc deleted the devsjc/timeseries-no-values branch April 28, 2026 13:47
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