samredai commented on issue #4435:
URL: https://github.com/apache/iceberg/issues/4435#issuecomment-1083982606


   Thanks for opening this @dramaticlly, to add more, I think we should section 
off the ABC classes to a `base.py` file (doesn't have to be the same PR as the 
general restructuring). Mainly because the expressions module has a pretty 
gnarly sequence of class inheritance that's hard to follow when the interfaces 
are scattered among the concrete classes. This also keeps the files with the 
actual logic easy to follow since the top of those files are not full of super 
classes. In turn that also makes it easier to find where pdb breakpoints need 
to be set while debugging.
   
   Here's what I imagine the file contents would look like:
   - **base.py** (ABC classes that are never instantiated directly)
     - Literal
     - Expression
     - ExpressionVisitor
     - BoundExpressionVisitor
     - Evaluator
     - Bound
     - Unbound
     - Term
     - Predicate
   - **literals.py**
     - IntegerLiteral
     - BooleanLiteral
     - DecimalLiteral
     - ...
   - **operation.py**
     - Operation
     - And
     - Or
     - Not
     - TrueExp
     - FalseExp
   - **predicate.py**
     - BoundPredicate
     - UnboundPredicate
   - **references.py**
     - Boundreference
     - UnboundReference
   - **visitors.py**
     - EvalVisitor
     - BindVisitor
     - ReferenceVisitor
     - ManifestEvalVisitor
     - MetricsEvalVisitor
   - **evaluators.py**
     - InclusiveManifestEvaluator
     - StrictMetricsEvaluator
   
   I may have left out a couple classes/functions but I think it captures the 
gist. There's of course a good amount of opinion in this but personally it 
helps navigate a pretty hefty expressions module. I think of it as "I need to 
look into an evaluator, must be in evaluator.py" or "I need to debug a visitor, 
it must be in visitors.py". If I want to get a better understanding of the 
interfaces and how they all come together, I go to `base.py` and find a 
collection of well-documented ABC classes.


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