DISCUSSION: Should deserialized timestamps have a timezone?

[dhermes]

Also relates to #278.

It's not immediately clear how we should support this.

The fact that we support datetime.datetime objects with timezones makes the seralize/deserialize operation potentially a function other than the identity.

For example

>>> from gcloud.datastore.datastore_v1_pb2 import Value
>>> value_pb = Value()
>>> import datetime
>>> import pytz
>>> timestamp = datetime.datetime.utcnow().replace(tzinfo=pytz.timezone('CET'))
>>> timestamp
datetime.datetime(2014, 10, 21, 23, 49, 31, 816554, tzinfo=<DstTzInfo 'CET' CET+1:00:00 STD>)
>>> from gcloud.datastore._helpers import _set_protobuf_value
>>> _set_protobuf_value(value_pb, timestamp)
>>> from gcloud.datastore._helpers import _get_value_from_value_pb
>>> _get_value_from_value_pb(value_pb)
datetime.datetime(2014, 10, 21, 22, 49, 31, 816554, tzinfo=<UTC>)

I realized this while doing a before/after dictionary comparison in the regression test

[pcostell]

As a point of reference, NDB supports datetime but doesn't allow any timezone set and assumes utc. This somewhat solves the problem because it only allows offset-naive datetimes.

[dhermes]

I am in favor of that approach.

We could avoid throwing away the good work by putting the offset -> naive conversion into datastore.helpers or by documenting the process in a place people will see.

This solves equality within the datetime.datetime type but we need a Property abstraction to map the difference between time.time, datetime.date and datetime.datetime.

[tseaver]

When marshalling naive datetime values to a protobuf value, we already convert them to microseconds (silently) as though they were explicitly UTC. When marshalling zoned datetime values, we first convert to UTC, and then compute the timestamp as microsecons.

The real question is what we should do when unmarshalling a microsecond timestamp from a protobuf: we know it must be UTC, so #181 explicitly adds the UTC zone to it before setting it into the entity dict (rather than leaving it ambiguously naive).

While we could marshall a 'datetime.date' to a timestamp (assuming midnight UTC), there is no way to unmarshall a microsecond timestamp as anything other than a 'datetime.datetime' instance. Given that it would be confusing to marshal a as one type but unmarshall another, I'm not sure we should support passing in 'datetime.date' (principle of least surprise).

[dhermes]

@tseaver I totally agree. This is too low-level to provide the needed information (i.e. an underlying type) as is done in some higher level libraries:
https://cloud.google.com/appengine/docs/python/ndb/properties#Date_and_Time

However, by requiring naive timestamps are passed in, we can at least guarantee the marshall / unmarshall operations are true inverse functions.

That's why I suggested decoupling the tz -> naive part into it's own helper method.

[dhermes]

Closing this out in light of closing comment of #278