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]