[GitHub] [iceberg] Fokko commented on a diff in pull request #5303: Add a BoundBooleanExpressionVisitor for visiting bound expressions

[GitBox <gi...@apache.org>]

Fokko commented on code in PR #5303:
URL: https://github.com/apache/iceberg/pull/5303#discussion_r925302436


##########
python/pyiceberg/expressions/base.py:
##########
@@ -327,30 +328,26 @@ def bind(self, schema: Schema, case_sensitive: bool) -> BoundIn[T]:
         return BoundIn(bound_ref, tuple(lit.to(bound_ref.field.field_type) for lit in self.literals))  # type: ignore
 
 
-class BooleanExpressionVisitor(Generic[T], ABC):

Review Comment:
   I would also prefer to keep it abstract. What's also common is to raise a `NotImplementedError`, see https://docs.python.org/3/library/exceptions.html#NotImplementedError. This way you still need to define all the methods, but you can just raise the exceptions on the methods that don't make any sense.



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