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


##########
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:
   Yes it would return `None`. What prompted me to do this is that 
`BoundBooleanExpressionVisitor` needs to inherit from `BooleanExpression` in 
order to use the `visit` function defined here. But that means it would need to 
override each method it doesn't use and return `None` (this is my understanding 
of what the Java one does).
   
   But I guess there's no reason that needs to be a default behavior from the 
parent class and it's better to explicitly define that for this visitor. I'll 
change this back and implement those methods.



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

Reply via email to