You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/08/02 02:58:16 UTC

[GitHub] [iceberg] jun-he opened a new pull request #2908: Support binary and fixed type in identity

jun-he opened a new pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908


   Start to update transforms package.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] jun-he commented on a change in pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
jun-he commented on a change in pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#discussion_r686256464



##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -59,7 +60,7 @@ def human_hour(hour_ordinal):
 
     @staticmethod
     def base_64_encode(buffer):
-        raise NotImplementedError()

Review comment:
       Updated.

##########
File path: python/iceberg/api/transforms/identity.py
##########
@@ -62,13 +62,12 @@ def to_human_string(self, value):
             else:
                 return TransformUtil.human_timestamp_without_timezone(value)
         elif self.type_var.type_id in (TypeID.BINARY, TypeID.FIXED):
-            raise NotImplementedError()
-            # if isinstance(value, bytearray):
-            #     return base64.b64encode(value)
-            # elif isinstance(value, bytes):
-            #     return base64.b64encode(bytes(value))
-            # else:
-            #     raise RuntimeError("Unsupported binary type: %s" % value.__class__.__name__)
+            if isinstance(value, bytearray):

Review comment:
       Thanks for the comment. Fixed.




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] jun-he commented on a change in pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
jun-he commented on a change in pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#discussion_r693408436



##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -15,70 +15,70 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import base64
 from datetime import datetime, timedelta
 
 import pytz
 
 
-class TransformUtil(object):
-    EPOCH = datetime.utcfromtimestamp(0)
-    EPOCH_YEAR = datetime.utcfromtimestamp(0).year
-
-    @staticmethod
-    def human_year(year_ordinal):
-        return "{0:0=4d}".format(TransformUtil.EPOCH_YEAR + year_ordinal)
-
-    @staticmethod
-    def human_month(month_ordinal):
-        return "{0:0=4d}-{1:0=2d}".format(TransformUtil.EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
-
-    @staticmethod
-    def human_day(day_ordinal):
-        day = TransformUtil.EPOCH + timedelta(days=day_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
-
-    @staticmethod
-    def human_time(micros_from_midnight):
-        day = TransformUtil.EPOCH + timedelta(microseconds=micros_from_midnight)
-        return "{}".format(day.time())
-
-    @staticmethod
-    def human_timestamp_with_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
-
-    @staticmethod
-    def human_timestamp_without_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return day.isoformat()
-
-    @staticmethod
-    def human_hour(hour_ordinal):
-        time = TransformUtil.EPOCH + timedelta(hours=hour_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
-
-    @staticmethod
-    def base_64_encode(buffer):
-        raise NotImplementedError()
-
-    @staticmethod
-    def diff_hour(date1, date2):
-        return int((date1 - date2).total_seconds() / 3600)
-
-    @staticmethod
-    def diff_day(date1, date2):
-        return (date1 - date2).days
-
-    @staticmethod
-    def diff_month(date1, date2):
-        return (date1.year - date2.year) * 12 + (date1.month - date2.month) - (1 if date1.day < date2.day else 0)
-
-    @staticmethod
-    def diff_year(date1, date2):
-        return (date1.year - date2.year) - \
-               (1 if date1.month < date2.month or (date1.month == date2.month and date1.day < date2.day) else 0)
-
-    @staticmethod
-    def unscale_decimal(decimal_value):
-        value_tuple = decimal_value.as_tuple()
-        return int(("-" if value_tuple.sign else "") + "".join([str(d) for d in value_tuple.digits]))
+EPOCH = datetime.utcfromtimestamp(0)
+EPOCH_YEAR = datetime.utcfromtimestamp(0).year
+
+
+def human_year(year_ordinal):
+    return "{0:0=4d}".format(EPOCH_YEAR + year_ordinal)
+
+
+def human_month(month_ordinal):
+    return "{0:0=4d}-{1:0=2d}".format(EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
+
+
+def human_day(day_ordinal):
+    day = EPOCH + timedelta(days=day_ordinal)
+    return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
+
+
+def human_time(micros_from_midnight):
+    day = EPOCH + timedelta(microseconds=micros_from_midnight)
+    return "{}".format(day.time())
+
+
+def human_timestamp_with_timezone(timestamp_micros):
+    day = EPOCH + timedelta(microseconds=timestamp_micros)
+    return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
+
+
+def human_timestamp_without_timezone(timestamp_micros):
+    day = EPOCH + timedelta(microseconds=timestamp_micros)
+    return day.isoformat()
+
+
+def human_hour(hour_ordinal):
+    time = EPOCH + timedelta(hours=hour_ordinal)
+    return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
+
+
+def base_64_encode(buffer):
+    return base64.b64encode(buffer).decode('utf-8')
+
+
+def diff_hour(date1, date2):

Review comment:
       SGTM. As this is just a simple refactor to move the static method to module method, it might be better to do it in a separate PR.




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] jun-he commented on pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
jun-he commented on pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#issuecomment-903183796


   > A few comments below. I: think the original commit for the PR (binary and fixed types) has been lost as this is all decimal and time arithmetic functions now.
   
   @rymurr Sorry about the confusion. The recent commits to address the comments by refactoring transform_util produced a lot of changes and then `identity` change is not obvious but it was still there.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rymurr commented on a change in pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#discussion_r689464459



##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -15,70 +15,70 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import base64
 from datetime import datetime, timedelta
 
 import pytz
 
 
-class TransformUtil(object):
-    EPOCH = datetime.utcfromtimestamp(0)
-    EPOCH_YEAR = datetime.utcfromtimestamp(0).year
-
-    @staticmethod
-    def human_year(year_ordinal):
-        return "{0:0=4d}".format(TransformUtil.EPOCH_YEAR + year_ordinal)
-
-    @staticmethod
-    def human_month(month_ordinal):
-        return "{0:0=4d}-{1:0=2d}".format(TransformUtil.EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
-
-    @staticmethod
-    def human_day(day_ordinal):
-        day = TransformUtil.EPOCH + timedelta(days=day_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
-
-    @staticmethod
-    def human_time(micros_from_midnight):
-        day = TransformUtil.EPOCH + timedelta(microseconds=micros_from_midnight)
-        return "{}".format(day.time())
-
-    @staticmethod
-    def human_timestamp_with_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
-
-    @staticmethod
-    def human_timestamp_without_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return day.isoformat()
-
-    @staticmethod
-    def human_hour(hour_ordinal):
-        time = TransformUtil.EPOCH + timedelta(hours=hour_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
-
-    @staticmethod
-    def base_64_encode(buffer):
-        raise NotImplementedError()
-
-    @staticmethod
-    def diff_hour(date1, date2):
-        return int((date1 - date2).total_seconds() / 3600)
-
-    @staticmethod
-    def diff_day(date1, date2):
-        return (date1 - date2).days
-
-    @staticmethod
-    def diff_month(date1, date2):
-        return (date1.year - date2.year) * 12 + (date1.month - date2.month) - (1 if date1.day < date2.day else 0)
-
-    @staticmethod
-    def diff_year(date1, date2):
-        return (date1.year - date2.year) - \
-               (1 if date1.month < date2.month or (date1.month == date2.month and date1.day < date2.day) else 0)
-
-    @staticmethod
-    def unscale_decimal(decimal_value):
-        value_tuple = decimal_value.as_tuple()
-        return int(("-" if value_tuple.sign else "") + "".join([str(d) for d in value_tuple.digits]))
+EPOCH = datetime.utcfromtimestamp(0)
+EPOCH_YEAR = datetime.utcfromtimestamp(0).year
+
+
+def human_year(year_ordinal):
+    return "{0:0=4d}".format(EPOCH_YEAR + year_ordinal)
+
+
+def human_month(month_ordinal):
+    return "{0:0=4d}-{1:0=2d}".format(EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
+
+
+def human_day(day_ordinal):
+    day = EPOCH + timedelta(days=day_ordinal)
+    return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
+
+
+def human_time(micros_from_midnight):
+    day = EPOCH + timedelta(microseconds=micros_from_midnight)
+    return "{}".format(day.time())
+
+
+def human_timestamp_with_timezone(timestamp_micros):
+    day = EPOCH + timedelta(microseconds=timestamp_micros)
+    return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
+
+
+def human_timestamp_without_timezone(timestamp_micros):
+    day = EPOCH + timedelta(microseconds=timestamp_micros)
+    return day.isoformat()
+
+
+def human_hour(hour_ordinal):
+    time = EPOCH + timedelta(hours=hour_ordinal)
+    return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
+
+
+def base_64_encode(buffer):
+    return base64.b64encode(buffer).decode('utf-8')
+
+
+def diff_hour(date1, date2):

Review comment:
       same with these, you could probably collapse this and changes to `timestamps.py` into simpler methods if we stuck to existing time libraries (I note they are already being created as timestamps anyways)

##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -15,70 +15,70 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import base64
 from datetime import datetime, timedelta
 
 import pytz
 
 
-class TransformUtil(object):
-    EPOCH = datetime.utcfromtimestamp(0)
-    EPOCH_YEAR = datetime.utcfromtimestamp(0).year
-
-    @staticmethod
-    def human_year(year_ordinal):
-        return "{0:0=4d}".format(TransformUtil.EPOCH_YEAR + year_ordinal)
-
-    @staticmethod
-    def human_month(month_ordinal):
-        return "{0:0=4d}-{1:0=2d}".format(TransformUtil.EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
-
-    @staticmethod
-    def human_day(day_ordinal):
-        day = TransformUtil.EPOCH + timedelta(days=day_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
-
-    @staticmethod
-    def human_time(micros_from_midnight):
-        day = TransformUtil.EPOCH + timedelta(microseconds=micros_from_midnight)
-        return "{}".format(day.time())
-
-    @staticmethod
-    def human_timestamp_with_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
-
-    @staticmethod
-    def human_timestamp_without_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return day.isoformat()
-
-    @staticmethod
-    def human_hour(hour_ordinal):
-        time = TransformUtil.EPOCH + timedelta(hours=hour_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
-
-    @staticmethod
-    def base_64_encode(buffer):
-        raise NotImplementedError()
-
-    @staticmethod
-    def diff_hour(date1, date2):
-        return int((date1 - date2).total_seconds() / 3600)
-
-    @staticmethod
-    def diff_day(date1, date2):
-        return (date1 - date2).days
-
-    @staticmethod
-    def diff_month(date1, date2):
-        return (date1.year - date2.year) * 12 + (date1.month - date2.month) - (1 if date1.day < date2.day else 0)
-
-    @staticmethod
-    def diff_year(date1, date2):
-        return (date1.year - date2.year) - \
-               (1 if date1.month < date2.month or (date1.month == date2.month and date1.day < date2.day) else 0)
-
-    @staticmethod
-    def unscale_decimal(decimal_value):
-        value_tuple = decimal_value.as_tuple()
-        return int(("-" if value_tuple.sign else "") + "".join([str(d) for d in value_tuple.digits]))
+EPOCH = datetime.utcfromtimestamp(0)
+EPOCH_YEAR = datetime.utcfromtimestamp(0).year
+
+
+def human_year(year_ordinal):
+    return "{0:0=4d}".format(EPOCH_YEAR + year_ordinal)
+
+
+def human_month(month_ordinal):
+    return "{0:0=4d}-{1:0=2d}".format(EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
+
+
+def human_day(day_ordinal):
+    day = EPOCH + timedelta(days=day_ordinal)
+    return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
+
+
+def human_time(micros_from_midnight):
+    day = EPOCH + timedelta(microseconds=micros_from_midnight)
+    return "{}".format(day.time())
+
+
+def human_timestamp_with_timezone(timestamp_micros):
+    day = EPOCH + timedelta(microseconds=timestamp_micros)
+    return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
+
+
+def human_timestamp_without_timezone(timestamp_micros):
+    day = EPOCH + timedelta(microseconds=timestamp_micros)
+    return day.isoformat()
+
+
+def human_hour(hour_ordinal):
+    time = EPOCH + timedelta(hours=hour_ordinal)
+    return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
+
+
+def base_64_encode(buffer):
+    return base64.b64encode(buffer).decode('utf-8')
+
+
+def diff_hour(date1, date2):
+    return int((date1 - date2).total_seconds() / 3600)
+
+
+def diff_day(date1, date2):
+    return (date1 - date2).days
+
+
+def diff_month(date1, date2):
+    return (date1.year - date2.year) * 12 + (date1.month - date2.month) - (1 if date1.day < date2.day else 0)
+
+
+def diff_year(date1, date2):
+    return (date1.year - date2.year) - \
+           (1 if date1.month < date2.month or (date1.month == date2.month and date1.day < date2.day) else 0)
+
+
+def unscale_decimal(decimal_value):

Review comment:
       similar to time manipulation it feels like we could do this w/ standard library methods rather than doing our own decimal arithmetic

##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -15,70 +15,70 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import base64
 from datetime import datetime, timedelta
 
 import pytz
 
 
-class TransformUtil(object):
-    EPOCH = datetime.utcfromtimestamp(0)
-    EPOCH_YEAR = datetime.utcfromtimestamp(0).year
-
-    @staticmethod
-    def human_year(year_ordinal):
-        return "{0:0=4d}".format(TransformUtil.EPOCH_YEAR + year_ordinal)
-
-    @staticmethod
-    def human_month(month_ordinal):
-        return "{0:0=4d}-{1:0=2d}".format(TransformUtil.EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
-
-    @staticmethod
-    def human_day(day_ordinal):
-        day = TransformUtil.EPOCH + timedelta(days=day_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
-
-    @staticmethod
-    def human_time(micros_from_midnight):
-        day = TransformUtil.EPOCH + timedelta(microseconds=micros_from_midnight)
-        return "{}".format(day.time())
-
-    @staticmethod
-    def human_timestamp_with_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
-
-    @staticmethod
-    def human_timestamp_without_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return day.isoformat()
-
-    @staticmethod
-    def human_hour(hour_ordinal):
-        time = TransformUtil.EPOCH + timedelta(hours=hour_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
-
-    @staticmethod
-    def base_64_encode(buffer):
-        raise NotImplementedError()
-
-    @staticmethod
-    def diff_hour(date1, date2):
-        return int((date1 - date2).total_seconds() / 3600)
-
-    @staticmethod
-    def diff_day(date1, date2):
-        return (date1 - date2).days
-
-    @staticmethod
-    def diff_month(date1, date2):
-        return (date1.year - date2.year) * 12 + (date1.month - date2.month) - (1 if date1.day < date2.day else 0)
-
-    @staticmethod
-    def diff_year(date1, date2):
-        return (date1.year - date2.year) - \
-               (1 if date1.month < date2.month or (date1.month == date2.month and date1.day < date2.day) else 0)
-
-    @staticmethod
-    def unscale_decimal(decimal_value):
-        value_tuple = decimal_value.as_tuple()
-        return int(("-" if value_tuple.sign else "") + "".join([str(d) for d in value_tuple.digits]))
+EPOCH = datetime.utcfromtimestamp(0)
+EPOCH_YEAR = datetime.utcfromtimestamp(0).year
+
+
+def human_year(year_ordinal):

Review comment:
       Why not use `strftime` everywhere? eg 
   
   ```
   from dateutil.relativedelta import relativedelta
   (EPOCH + relativedelta(years=year_ordinal)).strftime('%Y')
   ```
   
   I know it is a bit slower than your impl but I find date math so easy to screw up. I woudl rather not have to worry about it.




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] jun-he commented on pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
jun-he commented on pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#issuecomment-890673217


   @rymurr @TGooch44 Can you help to review it? Thanks.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] jun-he edited a comment on pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
jun-he edited a comment on pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#issuecomment-903183796


   > A few comments below. I: think the original commit for the PR (binary and fixed types) has been lost as this is all decimal and time arithmetic functions now.
   
   @rymurr Sorry about the confusion. The recent commits to address the comments by refactoring transform_util produced a lot of changes and then `identity` change is not obvious but it is still there.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] rymurr commented on a change in pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
rymurr commented on a change in pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#discussion_r680762075



##########
File path: python/iceberg/api/transforms/identity.py
##########
@@ -62,13 +62,12 @@ def to_human_string(self, value):
             else:
                 return TransformUtil.human_timestamp_without_timezone(value)
         elif self.type_var.type_id in (TypeID.BINARY, TypeID.FIXED):
-            raise NotImplementedError()
-            # if isinstance(value, bytearray):
-            #     return base64.b64encode(value)
-            # elif isinstance(value, bytes):
-            #     return base64.b64encode(bytes(value))
-            # else:
-            #     raise RuntimeError("Unsupported binary type: %s" % value.__class__.__name__)
+            if isinstance(value, bytearray):

Review comment:
       nit: why use two branches instead of an `or`

##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -59,7 +60,7 @@ def human_hour(hour_ordinal):
 
     @staticmethod
     def base_64_encode(buffer):
-        raise NotImplementedError()

Review comment:
       similar to the other PR this should probably be a set of functions rather than static methods in a class




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


[GitHub] [iceberg] jun-he commented on a change in pull request #2908: Support binary and fixed type in identity

Posted by GitBox <gi...@apache.org>.
jun-he commented on a change in pull request #2908:
URL: https://github.com/apache/iceberg/pull/2908#discussion_r693408375



##########
File path: python/iceberg/api/transforms/transform_util.py
##########
@@ -15,70 +15,70 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import base64
 from datetime import datetime, timedelta
 
 import pytz
 
 
-class TransformUtil(object):
-    EPOCH = datetime.utcfromtimestamp(0)
-    EPOCH_YEAR = datetime.utcfromtimestamp(0).year
-
-    @staticmethod
-    def human_year(year_ordinal):
-        return "{0:0=4d}".format(TransformUtil.EPOCH_YEAR + year_ordinal)
-
-    @staticmethod
-    def human_month(month_ordinal):
-        return "{0:0=4d}-{1:0=2d}".format(TransformUtil.EPOCH_YEAR + int(month_ordinal / 12), 1 + int(month_ordinal % 12))
-
-    @staticmethod
-    def human_day(day_ordinal):
-        day = TransformUtil.EPOCH + timedelta(days=day_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}".format(day.year, day.month, day.day)
-
-    @staticmethod
-    def human_time(micros_from_midnight):
-        day = TransformUtil.EPOCH + timedelta(microseconds=micros_from_midnight)
-        return "{}".format(day.time())
-
-    @staticmethod
-    def human_timestamp_with_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return pytz.timezone("UTC").localize(day).strftime("%Y-%m-%dT%H:%M:%S.%fZ")
-
-    @staticmethod
-    def human_timestamp_without_timezone(timestamp_micros):
-        day = TransformUtil.EPOCH + timedelta(microseconds=timestamp_micros)
-        return day.isoformat()
-
-    @staticmethod
-    def human_hour(hour_ordinal):
-        time = TransformUtil.EPOCH + timedelta(hours=hour_ordinal)
-        return "{0:0=4d}-{1:0=2d}-{2:0=2d}-{3:0=2d}".format(time.year, time.month, time.day, time.hour)
-
-    @staticmethod
-    def base_64_encode(buffer):
-        raise NotImplementedError()
-
-    @staticmethod
-    def diff_hour(date1, date2):
-        return int((date1 - date2).total_seconds() / 3600)
-
-    @staticmethod
-    def diff_day(date1, date2):
-        return (date1 - date2).days
-
-    @staticmethod
-    def diff_month(date1, date2):
-        return (date1.year - date2.year) * 12 + (date1.month - date2.month) - (1 if date1.day < date2.day else 0)
-
-    @staticmethod
-    def diff_year(date1, date2):
-        return (date1.year - date2.year) - \
-               (1 if date1.month < date2.month or (date1.month == date2.month and date1.day < date2.day) else 0)
-
-    @staticmethod
-    def unscale_decimal(decimal_value):
-        value_tuple = decimal_value.as_tuple()
-        return int(("-" if value_tuple.sign else "") + "".join([str(d) for d in value_tuple.digits]))
+EPOCH = datetime.utcfromtimestamp(0)
+EPOCH_YEAR = datetime.utcfromtimestamp(0).year
+
+
+def human_year(year_ordinal):

Review comment:
       @TGooch44  as you created those, wondering if the perf was a concern or the extra dateutil dependency was a concern?
   If not, we can consider to update most of places to use the public library.
   




-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org