zcsizmadia commented on code in PR #1634:
URL: https://github.com/apache/avro/pull/1634#discussion_r842260791
##########
lang/py/avro/io.py:
##########
@@ -362,25 +362,52 @@ def read_time_micros_from_long(self) -> datetime.time:
microseconds = self.read_long()
return self._build_time_object(microseconds, 1)
- def read_timestamp_millis_from_long(self) -> datetime.datetime:
+ def _read_timestamp_millis_from_long(self, tz:bool) -> datetime.datetime:
"""
long is decoded as python datetime object which represents
the number of milliseconds from the unix epoch, 1 January 1970.
"""
timestamp_millis = self.read_long()
timedelta = datetime.timedelta(microseconds=timestamp_millis * 1000)
- unix_epoch_datetime = datetime.datetime(1970, 1, 1, 0, 0, 0, 0,
tzinfo=avro.timezones.utc)
+ unix_epoch_datetime = datetime.datetime(1970, 1, 1, 0, 0, 0, 0,
tzinfo=avro.timezones.utc if tz else None)
return unix_epoch_datetime + timedelta
- def read_timestamp_micros_from_long(self) -> datetime.datetime:
+ def _read_timestamp_micros_from_long(self,tz:bool) -> datetime.datetime:
"""
long is decoded as python datetime object which represents
the number of microseconds from the unix epoch, 1 January 1970.
"""
timestamp_micros = self.read_long()
timedelta = datetime.timedelta(microseconds=timestamp_micros)
- unix_epoch_datetime = datetime.datetime(1970, 1, 1, 0, 0, 0, 0,
tzinfo=avro.timezones.utc)
+ unix_epoch_datetime = datetime.datetime(1970, 1, 1, 0, 0, 0, 0,
tzinfo=avro.timezones.utc if tz else None)
return unix_epoch_datetime + timedelta
+ def read_local_timestamp_micros_from_long(self) -> datetime.datetime:
Review Comment:
The way I read the Java implementation, the actual long is still the ms or
us from the utc epoch time, however th returned datetime is converted to local
time zone.
```
def read_local_timestamp_millis_from_long(self) -> datetime.datetime:
return self.read_timestamp_millis_from_long().astimezone()
def read_local_timestamp_micros_from_long(self) -> datetime.datetime:
return self.read_timestamp_micros_from_long().astimezone()
```
since `astimezone()` converts to local tz. And the original
`read_timestamp_micros/millis_from_long` functions can stay the same.
##########
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 write functions will be identical, just like the Java
implementation (hopefullt I will be corrected if I am wrong)
```
def write_local_timestamp_millis_long(self, datum: datetime.datetime) ->
None:
self.write_timestamp_millis_long(datum)
def write_local_timestamp_micros_long(self, datum: datetime.datetime) ->
None:
self.write_timestamp_micros_long(datum)
```
The original `write_timestamp_micros/millis_long` functions should stay the
same.
##########
lang/py/avro/test/test_schema.py:
##########
@@ -400,9 +400,10 @@ class InvalidTestSchema(TestSchema):
TIMEMICROS_LOGICAL_TYPE = [ValidTestSchema({"type": "long", "logicalType":
"time-micros"})]
-TIMESTAMPMILLIS_LOGICAL_TYPE = [ValidTestSchema({"type": "long",
"logicalType": "timestamp-millis"})]
-
+LOCALTIMESTAMPMICROS_LOGICAL_TYPE = [ValidTestSchema({"type": "long",
"logicalType": "local-timestamp-micros"})]
+LOCALTIMESTAMPMILLIS_LOGICAL_TYPE = [ValidTestSchema({"type": "long",
"logicalType": "local-timestamp-millis"})]
TIMESTAMPMICROS_LOGICAL_TYPE = [ValidTestSchema({"type": "long",
"logicalType": "timestamp-micros"})]
+TIMESTAMPMILLIS_LOGICAL_TYPE = [ValidTestSchema({"type": "long",
"logicalType": "timestamp-millis"})]
UUID_LOGICAL_TYPE = [ValidTestSchema({"type": "string", "logicalType":
"uuid"})]
Review Comment:
The unit tests should be added by copy pasting all the `ValidTestSchema ...`
tests in `test_schema.py`
and all the timestamp tests in `test_io.py` shoudl be duplicated for the
local timestamp.
--
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]
