csingley / ofxtools

Python OFX Library
Other
301 stars 68 forks source link

Preserve timezone when converting DateTime to OFX. #115

Closed stoklund closed 3 years ago

stoklund commented 3 years ago

Some OFX servers are particular about the formatting of the timezone in <DTSTART> elements, and they reject the [0:GMT] timezone produced by ofxtools.

Preserve the timezone of the datetime objects used when generating the OFX timestamps to avoid this problem.

See discussion in #114.

stoklund commented 3 years ago

Updated the PR with some tests, and also shared the formatting code with the Time class to make things consistent.

Some timezones in Australia and India have a fractional hour offset from UTC. The OFX 1.6 standard says:

Note that times zones are specified by an offset and optionally, a time zone name. The offset defines the time zone. Valid offset values are in the range from –12 to +12 for whole number offsets. Formatting is +12.00 to -12.00 for fractional offsets, plus sign may be omitted.

I interpret this as a decimal fraction, so India's +0530 offset becomes [+5.50:IST]. It looks like the existing TIME_REGEX interprets this as a +h.mm format containing minutes rather than a decimal fraction. It's not clear to me which interpretation is correct.

I don't know if OFX is used in India or Australia.

csingley commented 3 years ago

I interpret this as a decimal fraction, so India's +0530 offset becomes [+5.50:IST].

I don't think the OFX spec writers were precise enough with their language that their choice of the word "fraction" should outweigh the huge driving force to fall in line with ISO 8601 conventions.

The OFXv1 DTDs (SGML doc definitions) are sloppy, but the date & time regexes in ofxtools.Types were copied directly from the OFXv2 XML schema, which makes it explicit that the spec intends ISO 8601 semantics.

Don't worry about it though, usage of OFX outside of America is low. I'll fix this part.

Thanks very much for fixing up the tests, it's appreciated.

stoklund commented 3 years ago

I don't think the OFX spec writers were precise enough with their language that their choice of the word "fraction" should outweigh the huge driving force to fall in line with ISO 8601 conventions.

Yeah, I agree. Decimal fractions would be silly.

I pushed an updated version at https://github.com/stoklund/ofxtools/tree/timezone which uses minutes.

csingley commented 3 years ago

I was just sitting down to overhaul that old logic to use datetime division... thanks for doing it. A lot of code in this module dates back to the days of Python 2.

Now that we're requiring recent versions of Python, I guess we can now employ f-strings too!

csingley commented 3 years ago

Done in 8bee7fe

Thanks again.