CircArgs commented on code in PR #5258:
URL: https://github.com/apache/iceberg/pull/5258#discussion_r928970742
##########
python/pyiceberg/expressions/base.py:
##########
@@ -169,16 +171,64 @@ def bind(self, schema: Schema, case_sensitive: bool) ->
BoundReference[T]:
return BoundReference(field=field, accessor=accessor)
-@dataclass(frozen=True) # type: ignore[misc]
class BoundPredicate(Bound[T], BooleanExpression):
- term: BoundReference[T]
- literals: Tuple[Literal[T], ...]
+ def __init__(self, term: BoundTerm[T], literals: tuple[Literal[T], ...] |
Literal[T] | None = None):
+ self._term = term
+ self._literals = literals if isinstance(literals, tuple) else
(literals and (literals,))
+ self._validate_literals()
+
+ def _validate_literals(self):
+ if self.literals is None or len(self.literals) != 1:
+ raise AttributeError(f"{self.__class__.__name__} must have exactly
1 literal.")
+
+ @property
+ def term(self) -> BoundTerm[T]:
+ return self._term
+
+ @property
+ def literals(self) -> tuple[Literal[T], ...]:
+ return self._literals # type: ignore
+
+ def __str__(self) -> str:
+ return f"{self.__class__.__name__}({str(self.term)}{self.literals and
', '+str(self.literals)})"
+
+ def __repr__(self) -> str:
+ return f"{self.__class__.__name__}({repr(self.term)}{self.literals and
', '+repr(self.literals)})"
+
+ def __eq__(self, other) -> bool:
+ return id(self) == id(other) or (
+ type(self) == type(other) and self.term == other.term and
self.literals == other.literals
+ )
-@dataclass(frozen=True) # type: ignore[misc]
class UnboundPredicate(Unbound[T, BooleanExpression], BooleanExpression):
- term: Reference[T]
- literals: Tuple[Literal[T], ...]
+ def __init__(self, term: UnboundTerm[T], literals: tuple[Literal[T], ...]
| Literal[T] | None = None):
+ self._term = term
+ self._literals = literals if isinstance(literals, tuple) else
(literals and (literals,))
+ self._validate_literals()
+
+ def _validate_literals(self):
+ if self.literals is None or len(self.literals) != 1:
+ raise AttributeError(f"{self.__class__.__name__} must have exactly
1 literal.")
+
+ @property
+ def term(self) -> UnboundTerm[T]:
+ return self._term
+
+ @property
+ def literals(self) -> tuple[Literal[T], ...]:
+ return self._literals # type: ignore
+
+ def __str__(self) -> str:
+ return f"{self.__class__.__name__}({str(self.term)}{self.literals and
', '+str(self.literals)})"
+
+ def __repr__(self) -> str:
+ return f"{self.__class__.__name__}({repr(self.term)}{self.literals and
', '+repr(self.literals)})"
+
+ def __eq__(self, other) -> bool:
Review Comment:
I originally used dataclasses throughout a lot of this code, but had
difficulties getting things to play nice with the validation of literals. I
think the trouble comes from leveraging `__post_init__` in a similar way to how
`__init__` is currently, but it seems child classes don't care about the
parent's `__post_init__` hence why I switched to `__init__` and just work
things with `super`
--
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]
