Fix time representation and timezone issues

[ZeevG]

The way times (and dates) are handled is inconsistent and needs an overhaul.

Times are represented using a mix Python datetime objects and unix timestamps. This needs to be standardised to Python objects. Also, when using times, it is not clear what time zone the API expects. Is it local time or the time at the forecast location? This all needs to be cleaned up and documented.

One option (this is the approach that Forecast.io has taken) is to always use UTC time.

This is probably my prefered approach as it will standardise the representation of time within the wrapper and also between the wrapper and the HTTP API.

[jetheurer]

As I understand, a request is made to forecast.io with a time in the past, say 1/1/2014, data is returned from midnight on Jan 1 to midnight on Jan 2 in local time. There are no forecasts, just actual values. However when I make a request through python-forecastio, I am returned dates that d'ont make sense.

For example requesting data from New York for 1/1/2014, I get date ranges from 2013-12-31 21:00:00, ..., 2014-01-01 20:00:00

From forecast.io, I get: 1388552400 to 1388635200 01 / 01 / 14 @ 5:00:00am UTC to 01 / 02 / 14 @ 4:00:00am UTC

[alexlouden]

Maybe look at using https://github.com/rhettg/dmc?

[Marco-Santoni]

I have exactly the same problem as @jetheurer. If I make a request for a certain datetime, e.g. 1/1/14, the daily forecast is actually for the previous date.

[ZeevG]

Thanks for letting me know. I am aware of this problem and the plan is to fix it using dmc (https://github.com/rhettg/dmc) when I release version 2.0 which will be soon.

[hack-c]

Hey, @ZeevG @alexlouden @jetheurer @Marco-Santoni --

My understanding of the issue:

The .load_forecast() method is currently converting the datetime argument to seconds since the epoch in local time, which has the effect of making it an "aware" datetime. That is, it imbues the datetime with statefulness w.r.t. location that it doesn't necessarily have already, if it's passed as a naïve datetime (i.e. "5 o'clock somewhere", not necessarily "5 o'clock in Greenwich.")

Technically, the issue is that time.mktime() takes an argument in local time (i.e. the locale of my laptop) and converts to seconds since the epoch (docs). In the API though, they expect a naïve datetime, unless timezone is specified. From the API docs:

For the latter format, if no timezone is present, local time (at the provided latitude and longitude) is assumed. (This string format is a subset of ISO 8601 time. An as example, 2013-05-06T12:00:00-0400.)

My thought was, that in order to make the wrapper as thin as possible, that it should behave in the same way. Instead of using mktime(), it should use isoformat().

Timezones are kind of a mindf!ck hahah, sometimes it's easier to show:

In [1]: %paste
import time
import datetime
from delorean import Delorean

## -- End pasted text --

In [2]: d = datetime.datetime(2015, 4, 11, 2, 45)  # initialized without any tzinfo

In [3]: d.isoformat()  # this format has no tzinfo, is naïvely unaware of location
Out[3]: '2015-04-11T02:45:00'

In [4]: ep = time.mktime(d.timetuple())

In [5]: ep  # this number is only associated with datetime(2015, 4, 11, 2, 45) in the sense of GMT-04:00 (I'm in New York.) So it's no longer naïve to location.
Out[5]: 1428734700.0

In [6]: dl = Delorean(d, timezone="US/Mountain")  # use delorean to create a timezone-aware datetime.datetime

In [7]: dl.datetime  
Out[7]: datetime.datetime(2015, 4, 11, 2, 45, tzinfo=<DstTzInfo 'US/Mountain' MDT-1 day, 18:00:00 DST>)

In [8]: dl.datetime.isoformat()  # in this case timezone info would be passed on to the API.
Out[8]: '2015-04-11T02:45:00-06:00'

Thanks again man! Hope this makes sense. I've made the change locally but I think you need to delegate me push access in order for me to make a pull request.

Cheers from NY,

Charlie Hack

/cc @jwthomas04

[ZeevG]

Thanks for the help @hack-c

This has been a problem for a while. I have been working on version 2.0 which refreshes the load_forecast method, makes it a lot nicer and will fix this problem. See what I've been working on here. https://github.com/ZeevG/python-forecast.io/blob/2.0.0/forecastio/api.py#L10

I'm not sure what changes you made. If you fork the repo and work on your fork, then I should be able to take a look at them. However I could use calendar.timegm which is the UTC time.mktime() equivalent. I think this should sort out most of the problems.

If this was the case, what would I do if users passed a timezone aware datetime or a datetime which is not in UTC? Throw an exception maybe?

[hack-c]

Hey @ZeevG,

V2.0 is looking good! So, in this line, I guess the datetime >> string formatting hasn't happened yet?

My PR uses isoformat() because the API expects it, and it preserves the concept of a naïve datetime (which the user expects Forecast to understand as local time at (lat, lng), as documented.

I guess how to parse the user's time input is really a higher-order question about design though-- how much "work" do we want the wrapper to do?

My preference is generally to keep wrappers very thin, so that the user barely notices they're there; which is why I hesitate to do much in the way of conversions or processing of the user input data.

However, there is a credible argument to be made, that the wrapper could function as more than just an abstraction for making the web calls, and move some processing over to the client as well. The geocoding is a good example of where this strategy is very useful.

So I guess my vote goes to "be sparing when transforming input data, unless the user explicitly asks for it." :)

[ZeevG]

Okay, I have these issues sorted! Timezones can be super confusing :confused: But I've been doing a lot of testing and the changes from @hack-c and @bzillins have everything working correctly.

I will also be updating the docs to reflect these changes.

[dswah]

hi there, im still a horrible time interpreting the times in the response etc... (im using 1.3.5)

when is 2.0 coming out?

[dswah]

[UPDATE] well now im understanding it better.

it seems that forecastio takes your requested tz-aware datetime and snaps it to the beginning of that corresponding date at the requested place?