Open drmrbrewer opened 2 years ago
Thanks for opening this- to be honest, getting time zones sorted out was like 20% of the challenge of this entire project!
I've puzzled over the same set of documents, and came to the same conclusion here. I just ignored offset for this project, since Dark Sky has officially dropped it, And you're right that the "time" response is always and forever in UTC, regardless of anything else. I've tagged this as documentation to remind myself to update the docs to clarify this though!
The 1 hour offset is interesting though. The "day" should start at midnight local time, so if we're requesting "Monday, May 24 at 12:00" (1621857600), midnight local is 4:00 am UTC. This is what Dark Sky returns (1621828800), but PirateWeather is off by an hour, which is an issue! I'm almost sure it's a daylight savings time problem, since January 24th (outside of DST) works correctly!
Thanks again for noticing this! I fixed the daylight savings time bug (it was ignoring it, hence the issue), so that's a great catch! I also fixed the exclude tags. Right now, only the daily tag is working, but I'll get the rest added in shortly! It's currently implemented to just blank the response as a temporary fix, but I'd rather actually skip the processing steps and ignore it in the reply, since that will be closer to the Dark Sky convention.
So the darksky time
needs to be adjusted based on the offset
(pirateTime == darkskyTime + offset * 3600
)? Timezones and DST tend to blow my mind... I always cling on to the hope that a UTC time is the absolute gold standard, from which we determine local time (based on timezone + DST). But then the darksky time
is not actually UTC? It's only UTC after adjusting it by offset
?
also fixed the exclude tags
I'm still getting a daily
element in the json, just with an empty array within a data
sub-element... is that the intention?
With darksky's API, there is no daily
element at all.
I definitely need to improve the documentation here!
It's my understanding (and the implementation in PW) that UNIX timestamps are always in UTC, and that's how Dark Sky returns them. This aligns with their responses as well, since the daily blocks start at the UTC time that corresponds to midnight local. The offset is an entirely different thing, since it essentially returns the time zone offset of the point, which might be useful for data display or something?
Also, file the "daily" element as a "known issue" for now. I've got all the pieces there, but need to work out how to return a json.dumps
with changing elements, which is harder than it should be
I guess I have a hard time with "known issues", since I couldn't leave it like that. Looked into it some more, and json.dumps
just takes dicts, so that was easier than I though! exclude is now properly excluded in time machine, and I'll be extending it to the forecast side of things shortly!
Nice work! So time
really should be UTC, and offset
gives the information required in order to get to local time? I guess I'm slightly confused why data returned for the same query sent to darksky and pirate doesn't have the same time
for the first time point, then?
Pirate:
DarkSky:
These are one hour apart... the time
should be the same?
Also, I think that the values returned by pirate could benefit from being rounded e.g. to 2 d.p.?
I've noticed a couple of issues/discrepancies with the time machine data returned by pirateweather.
Take the following equivalent calls to pirateweather and darksky, respectively:
Firstly, I note that, despite passing
exclude=daily
to the pirateweather API, adaily
block is included in the return data.Secondly...
The start and end
time
in thehourly
block differs slightly between the two APIs... they are an hour apart? And indeed theoffset
value from pirateweather is0
whereas from darksky it is1
. That said, it's confusing because darksky's own docs suggest thatoffset
is deprecated:So maybe darksky (time machine) is doing it wrong, and shouldn't have anything non-zero for
offset
? And maybe I've been incorrectly ignoring this value, on the assumption thattime
was absolute and independent of timezone or DST (I use another library to convert from UTC to local time):