opendoor-labs / rets

RETS Python 3 Client
MIT License
89 stars 43 forks source link

Don't convert timezone naive dates to timezone aware #15

Open timworx opened 7 years ago

timworx commented 7 years ago

I noticed this behavior while dealing with an MLS that appears to be using EST for it's time.

However there is no system TimeZoneOffset and the actual dates are coming through as timezone naive ISO strings - 2017-07-14T16:47:57.210.

The rets package appears to be adding UTC timezone to the timezone naive strings. To me, this looks like an issue with their implementation, as they should really utilize TimeZoneOffset. Nonetheless, I don't see anything in the RETS docs about treating dates without timezone as UTC.

What are you thoughts on this?

martinxsliu commented 7 years ago

Thanks for flagging this issue! Timezone behaviour hasn't been something I've given a lot of thought towards.

You're right that the RETS doc doesn't seem to explicitly specify what the timezone of returned dates ought to be, however it does make a mention of timezones for dates in the search query. (e.g. see https://github.com/troydavisson/PHRETS/wiki/Timezone-Handling). The part that stands out to me is (in section 7.7.2):

If the time offset is not declared in the query, the server system MUST interpret the request using the default System time zone offset. This default must match the advertised time zone offset of the SYSTEM-METADATA. If no time zone offset is advertised for the server system system, the default time zone offset MUST be UTC.

This seems to suggest that if the TimeZoneOffset is omitted, then the system timezone and returned dates is UTC. Caveat that I may be reading too much into this, and of course it is up to the MLS to adhere to this as you point out.

Separately, this brings up another issue that if the MLS does advertise aTimeZoneOffset, then the current parsing does not take this into account – it only parses the date string according to RFC3339 and nothing more. Should we add the system timezone to such parsed timestamps? And what should be behaviour be if the date string contains a different timezone offset?

timworx commented 7 years ago

That tends to be the case with timezone stuff. It's very out of sight out of mind, until it bites and you have to deal with it. :P

My general thinking with this would be:

RETS doesn't give much help here. The snippet you mention above is somewhat ambiguous:

If no time zone offset is advertised for the server system system, the default time zone offset MUST be UTC.

I'm not sure it it's just talking about queries, as it is in the sentences before it, or the data returned as well.

In the RETS implementation I'm currently working with there is not offset specified and the dates are tz naive. However, they're being serialized by the library as UTC which causes some quiet bugs.

mlucic commented 7 years ago

Contingent on the RETS server supporting date queries using alternate timezones (e.g. 2017-11-23T16:50:00-05:00), if you retrieve a listing by querying for it via its modification timestamp with a specific timezone, then run a second query for the same date/time but altering it by an hour and changing the timezone to compensate, you can compare a date field between the two listings to determine the timezone being used by date values returned by the server, and whether the timezone in the query affects the returned values timezone.

E.g. First query -> (ModificationTimestamp=2017-01-01T12:00:00+04:00) Second query -> (ModificationTimestamp=2017-01-01T13:00:00+03:00)