Make sure we respect local system time #61
Merged
Conversation
This file contains 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
This probably needs to be done on an api-interface by api-interface basis (as how we do it depends on how we convert whatever comes back into a datetime object). For the ciuk we have explicit UTC returned but strptime does not know how to parse this. So add the timezone info for each datetime object. Also modify the test to make sure all datetime objects we get back from the api_query are aware of their datetime (we don't care what this is, only that it is defined). We can then convert to system time at the other end.
Make all test data going into the unit tests for WindowedForcast have a timezone (specifically UTC) and add that timezone to the expected output. The existing assert statments will fail if timezone data does not match.
As far as I can tell, when using the posix timestamp to at, it works in local system time (which for me is BST). So make all our start times and reports from the optimiser timezone aware and in local system time. It turns out that all we need to do to do this is feed in a start time that is timezone aware and in the right timezone. Doing this shifts the time that gets passed generated for feeding into at by 1 hr, which is what we want. Also add a test case to test_windowed_forecast to make sure that this all works. Note that we have to explicitly check the timezones as the same time with different timezones compare equal.
andreww
requested review from
tlestang,
colinsauze,
ljcolling and
Llannelongue
|
I think this has everything we need to run correctly on systems where the system time is not UTC but the forecast data is. |
This documents the need for two checks for timezone awarness, and makes one of the more complex forecast checks cross timezones between the data and 'system' time.
tlestang
approved these changes
Aug 11, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This (draft) pull request makes sure we keep track of any differences in timezone between system time (e.g. used for
at) and the time zone returned by our datasource. To do this, we need to:With a little luck, the actual calculation will 'just work'. We should check this.
This addresses #21