kojiromike commented on code in PR #1634:
URL: https://github.com/apache/avro/pull/1634#discussion_r912406605
##########
lang/py/avro/io.py:
##########
@@ -586,26 +613,56 @@ def write_time_micros_long(self, datum: datetime.time) ->
None:
def _timedelta_total_microseconds(self, timedelta_: datetime.timedelta) ->
int:
return timedelta_.microseconds + (timedelta_.seconds + timedelta_.days
* 24 * 3600) * 10**6
- def write_timestamp_millis_long(self, datum: datetime.datetime) -> None:
+ def _write_timestamp_millis_long(self, datum: datetime.datetime,
tz:bool)->None:
"""
Encode python datetime object as long.
It stores the number of milliseconds from midnight of unix epoch, 1
January 1970.
"""
- datum = datum.astimezone(tz=avro.timezones.utc)
- timedelta = datum - datetime.datetime(1970, 1, 1, 0, 0, 0, 0,
tzinfo=avro.timezones.utc)
+ if tz:
+ datum = datum.astimezone(tz=avro.timezones.utc)
+ timedelta = datum - datetime.datetime(1970, 1, 1, 0, 0, 0, 0,
tzinfo=avro.timezones.utc if tz else None)
milliseconds = self._timedelta_total_microseconds(timedelta) // 1000
self.write_long(milliseconds)
- def write_timestamp_micros_long(self, datum: datetime.datetime) -> None:
Review Comment:
I think the distinction is important.
If the datum is naïve (has no tzinfo), then `datum - datetime.datetime(1970,
1, 1, 0, 0, 0, 0, tzinfo=avro.timezones.utc)` will fail with `TypeError: can't
subtract offset-naive and offset-aware datetimes`.
On the other hand, if the datum has `tzinfo`, but the type is local, then
the old implementation will happily encode it, including the difference between
the timezone and UTC in the result. I think that would most likely be done in
error. A datetime with a timezone is an inappropriate datum for encoding in a
localtimestamp logical type. In my implementation, this would crash (although
I'm considering adding a more friendly error message and type).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
