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 2022/06/28 01:41:47 UTC

[GitHub] [iceberg] kbendick commented on a diff in pull request #5030: Python: Add truncate transform

kbendick commented on code in PR #5030:
URL: https://github.com/apache/iceberg/pull/5030#discussion_r907942355


##########
python/src/iceberg/transforms.py:
##########
@@ -301,6 +301,86 @@ def _(self, value_type: IcebergType, value: int) -> str:
         return datetime.to_human_timestamptz(value)
 
 
+class TruncateTransform(Transform[S, S]):
+    """A transform for truncating a value to a specified width.
+    Args:
+      source_type (Type): An Iceberg Type of IntegerType, LongType, StringType, BinaryType or DecimalType
+      width (int): The truncate width
+    Raises:
+      ValueError: If a type is provided that is incompatible with a Truncate transform
+    """
+
+    def __init__(self, source_type: IcebergType, width: int):
+        super().__init__(
+            f"truncate[{width}]",
+            f"transforms.truncate(source_type={repr(source_type)}, width={width})",
+        )
+        self._type = source_type
+        self._width = width
+
+    @property
+    def width(self):
+        return self._width
+
+    def apply(self, value: Optional[S]) -> Optional[S]:
+        return self._truncate_value(value) if value is not None else None
+
+    @singledispatchmethod
+    def _truncate_value(self, value: S) -> S:
+        raise ValueError(f"Cannot truncate value: {value}")
+
+    @_truncate_value.register(int)
+    def _(self, value):
+        """Truncate a given int value into a given width if feasible."""
+        if type(self._type) in {IntegerType, LongType}:

Review Comment:
   +1 to not validating inside of the right loop.
   
   I’d like to see general validation of the `source_type` (though I guess singledispatch takes care of that somewhat, it would be nice to see it a bit more explicitly but that’s just my opinion),
   
   Also validating in the constructor or elsewhere that the `width` is greater than zero.
   
   But this function will be called in a tight loop and it’s best to avoid expensive checks here.



-- 
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