[ 
https://issues.apache.org/jira/browse/AVRO-3476?focusedWorklogId=752603&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-752603
 ]

ASF GitHub Bot logged work on AVRO-3476:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Apr/22 01:35
            Start Date: 05/Apr/22 01:35
    Worklog Time Spent: 10m 
      Work Description: 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.





Issue Time Tracking
-------------------

    Worklog Id:     (was: 752603)
    Time Spent: 20m  (was: 10m)

> Python implementation does not support local-timestamp-micros and 
> local-timestamp-millis
> ----------------------------------------------------------------------------------------
>
>                 Key: AVRO-3476
>                 URL: https://issues.apache.org/jira/browse/AVRO-3476
>             Project: Apache Avro
>          Issue Type: Bug
>          Components: python
>    Affects Versions: 1.11.0
>            Reporter: Eric
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 20m
>  Remaining Estimate: 0h
>
> Even though [they are present in the 
> spec|https://avro.apache.org/docs/1.11.0/spec.html#Local+timestamp+(millisecond+precision)],
>  the Python implementation does not support logical types 
> local-timestamp-micros and local-timestamp-millis.
> When trying to use them, you get the following:
> {code:python}
> .../lib/python3.8/site-packages/avro/schema.py:1048: IgnoredLogicalType: 
> Unknown local-timestamp-micros, using long.
> {code}



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to