Skip to content

Daylight saving time offset on daily data #114

Description

@guillaume

Describe the bug

When we observe daylight saving time during the week, the daily times become offset

% curl -s "https://api.pirateweather.net/forecast/${PIRATE_KEY}/37.3895979,-1.9450652,1698357600?&units=si&extend=hourly&tz=precise"  | jq '.daily.data[].time'  | xargs -I {} env TZ=Europe/Madrid date -r {}
Fri 27 Oct 2023 00:00:00 CEST
Sat 28 Oct 2023 00:00:00 CEST
Sun 29 Oct 2023 00:00:00 CEST
Sun 29 Oct 2023 23:00:00 CET
Mon 30 Oct 2023 23:00:00 CET
Tue 31 Oct 2023 23:00:00 CET
Wed  1 Nov 2023 23:00:00 CET
Thu  2 Nov 2023 23:00:00 CET

Expected behavior

The daily[].time should reflect midnight for that day

Actual behavior

When there is a DST switch, the beginning of day is an hour off (23:00:00 of the previous day)

API Endpoint

Production

Location

Huércal-Overa, Almería, Spain used in the example but not limited to..

Other details

Possible to workaround and recompute using the hourly data

Troubleshooting steps

  • I have searched this repository and Home Assistant Repository to see if the issue has already been reported.
  • I have read through the API documentation before opening this issue.
  • I have written an informative title.

Activity

  1. cloneofghosts commented on Oct 30, 2023

    @cloneofghosts
    Collaborator

    Now that DST is coming to an end in North America this weekend I'm seeing the same issue that you've outlined. It's probably showing midnight daylight time but converting it into standard time which is causing the error. I would think after the switch everything displays fine?

    I'll ping @alexander0042 to take a look but it might not be fixed until the 2.0 release in the winter.

  2. alexander0042 commented on Nov 8, 2023

    @alexander0042
    Collaborator

    @cloneofghosts is spot on here, the fundamental issue is pretty simple. The script picks a zero hour using the time zone, then propagates forward in 24h increments. Since hourly data is continuous, it's only an issue for daily responses.

    Adding this to the project board for V2.0, since I've missed the DST window for this year. My plan for a fix is pretty simple- instead of making the midnight array by adding 24 hours, I'm going to convert the entire time series into local and then find the midnight indexes

  3. moved this from Todo to No status in Pirate Weather API 2.0on Nov 19, 2023
  4. github-actions commented on Mar 11, 2024

    @github-actions
  5. cloneofghosts commented on Mar 11, 2024

    @cloneofghosts
    Collaborator

    I've tagged this one as keep so it won't go stale in the future. From what I can tell this issue hasn't been fixed yet on the V2 API but still on the board to be fixed.

  6. alexander0042 commented on Apr 5, 2024

    @alexander0042
    Collaborator

    Finally fixed this- it was harder than it should have been, since timezone objects don't really like changing dst after they have been created. Just release a v2.0i to the dev channel, so should see it next time we have a DST switchover!

    For anyone trying to figure this out in the future, the trick is to create a new timezone object for each step, like this:

        day_array_grib =  np.array([pytzTZ.localize(datetime.datetime(year=baseTime.year,
                                  month=baseTime.month, day=baseTime.day) + datetime.timedelta(days=i)).astimezone(utc).timestamp() for i in range(9)]).astype(np.int32)
    

    where pytzTZ is pytz.timezone(tzSTRING)

  7. cloneofghosts commented on Apr 5, 2024

    @cloneofghosts
    Collaborator

    Will close this issue for now and we can always re-open the next time DST rolls around if it isn't fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingkeep

    Type

    No type

    Projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions