rdblue commented on code in PR #5124:
URL: https://github.com/apache/iceberg/pull/5124#discussion_r918106533
##########
python/pyiceberg/transforms.py:
##########
@@ -96,6 +127,38 @@ def dedup_name(self) -> str:
def __str__(self) -> str:
return self.__root__
+ def __eq__(self, other: Any) -> bool:
+ if isinstance(other, Transform):
+ return self.__root__ == other.__root__
+ return False
+
+
+class UnboundTransform(Transform):
Review Comment:
Yes, this definitely works.
My point was just that we could structure transforms slightly differently,
so that we don't need to bind in most situations. The only time we actually
need to get the concrete source type for a transform is when we are calling
`apply`. The other method signatures pass a type in. For example,
`can_transform(IcebergType)` doesn't need to be specific to a `LongType` -- we
can write a version for `BucketTransform` instead of for
`BucketNumberTransform`.
In that case, we'd change the code so that instead of calling `apply` on
`BucketNumberTransform`, we'd call `BucketTransform.bind` to get `Function[int,
int]` that implements the hash and bucketing logic.
That refactor may end up being easier to work with in the end because there
would be far fewer places where we'd need to bind the transform to a specific
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
