Fokko commented on code in PR #6139:
URL: https://github.com/apache/iceberg/pull/6139#discussion_r1024505483
##########
python/pyiceberg/expressions/__init__.py:
##########
@@ -194,85 +251,108 @@ def __new__(cls, child: BooleanExpression):
return AlwaysTrue()
elif isinstance(child, Not):
return child.child
- result = super().__new__(cls)
- object.__setattr__(result, "child", child)
- return result
+ obj = super().__new__(cls)
+ obj.child = child
+ return obj
+
+ def __repr__(self) -> str:
+ return f"Not(child={repr(self.child)})"
+
+ def __eq__(self, other: Any) -> bool:
+ return self.child == other.child if isinstance(other, Not) else False
def __invert__(self) -> BooleanExpression:
return self.child
-@dataclass(frozen=True)
class AlwaysTrue(BooleanExpression, Singleton):
"""TRUE expression"""
def __invert__(self) -> AlwaysFalse:
return AlwaysFalse()
+ def __str__(self):
+ return "AlwaysTrue()"
+
+ def __repr__(self):
+ return "AlwaysTrue()"
+
-@dataclass(frozen=True)
class AlwaysFalse(BooleanExpression, Singleton):
"""FALSE expression"""
def __invert__(self) -> AlwaysTrue:
return AlwaysTrue()
+ def __str__(self):
+ return "AlwaysFalse()"
+
+ def __repr__(self):
+ return "AlwaysFalse()"
-@dataclass(frozen=True)
-class BoundPredicate(Generic[T], Bound, BooleanExpression):
- term: BoundTerm[T]
- def __invert__(self) -> BoundPredicate[T]:
- """Inverts the predicate"""
- raise NotImplementedError
+class BoundPredicate(Generic[L], Bound, BooleanExpression, ABC):
+ term: BoundTerm[L]
+ def __init__(self, term: BoundTerm[L]):
+ self.term = term
-@dataclass(frozen=True)
-class UnboundPredicate(Generic[T], Unbound[BooleanExpression],
BooleanExpression):
- as_bound: ClassVar[type]
- term: UnboundTerm[T]
+ def __eq__(self, other: Any) -> bool:
+ if isinstance(other, BoundPredicate):
+ return self.term == other.term
+ return False
- def __invert__(self) -> UnboundPredicate[T]:
- """Inverts the predicate"""
- raise NotImplementedError
- def bind(self, schema: Schema, case_sensitive: bool = True) ->
BooleanExpression:
- """Binds the predicate to a schema"""
- raise NotImplementedError
+class UnboundPredicate(Unbound, BooleanExpression, ABC):
+ term: UnboundTerm
+ def __init__(self, term: Union[str, UnboundTerm]):
+ self.term = _to_unbound_term(term)
+
+ def __eq__(self, other):
+ return self.term == other.term if isinstance(other, UnboundPredicate)
else False
+
+ @abstractmethod
+ def bind(self, schema: Schema, case_sensitive: bool = True) ->
BoundPredicate:
+ ...
-@dataclass(frozen=True)
-class UnaryPredicate(UnboundPredicate[T]):
- def bind(self, schema: Schema, case_sensitive: bool = True) ->
BooleanExpression:
+ @property
+ @abstractmethod
+ def as_bound(self) -> Type[BoundPredicate]:
+ ...
+
+
+class UnaryPredicate(UnboundPredicate, ABC):
Review Comment:
`UnboundPredicate` doesn't have a Type, but I'm happy to revert that, and
then we can add `Any` here indeed. Reverted that 👍🏻
##########
python/pyiceberg/expressions/__init__.py:
##########
@@ -281,249 +361,313 @@ def __invert__(self) -> BoundIsNull:
return BoundIsNull(self.term)
-@dataclass(frozen=True)
-class IsNull(UnaryPredicate[T]):
- as_bound = BoundIsNull
-
- def __invert__(self) -> NotNull[T]:
+class IsNull(UnaryPredicate):
+ def __invert__(self) -> NotNull:
return NotNull(self.term)
+ @property
+ def as_bound(self) -> Type[BoundIsNull[L]]:
+ return BoundIsNull[L]
-@dataclass(frozen=True)
-class NotNull(UnaryPredicate[T]):
- as_bound = BoundNotNull
- def __invert__(self) -> IsNull[T]:
+class NotNull(UnaryPredicate):
+ def __invert__(self) -> IsNull:
return IsNull(self.term)
+ @property
+ def as_bound(self) -> Type[BoundNotNull[L]]:
+ return BoundNotNull[L]
-@dataclass(frozen=True)
-class BoundIsNaN(BoundUnaryPredicate[T]):
- def __new__(cls, term: BoundTerm[T]): # pylint: disable=W0221
+
+class BoundIsNaN(BoundUnaryPredicate[L]):
+ def __new__(cls, term: BoundTerm[L]): # pylint: disable=W0221
bound_type = term.ref().field.field_type
if type(bound_type) in {FloatType, DoubleType}:
return super().__new__(cls)
return AlwaysFalse()
- def __invert__(self) -> BoundNotNaN[T]:
+ def __invert__(self) -> BoundNotNaN[L]:
return BoundNotNaN(self.term)
-@dataclass(frozen=True)
-class BoundNotNaN(BoundUnaryPredicate[T]):
- def __new__(cls, term: BoundTerm[T]): # pylint: disable=W0221
+class BoundNotNaN(BoundUnaryPredicate[L]):
+ def __new__(cls, term: BoundTerm[L]): # pylint: disable=W0221
bound_type = term.ref().field.field_type
if type(bound_type) in {FloatType, DoubleType}:
return super().__new__(cls)
return AlwaysTrue()
- def __invert__(self) -> BoundIsNaN[T]:
+ def __invert__(self) -> BoundIsNaN[L]:
return BoundIsNaN(self.term)
-@dataclass(frozen=True)
-class IsNaN(UnaryPredicate[T]):
- as_bound = BoundIsNaN
-
- def __invert__(self) -> NotNaN[T]:
+class IsNaN(UnaryPredicate):
+ def __invert__(self) -> NotNaN:
return NotNaN(self.term)
+ @property
+ def as_bound(self) -> Type[BoundIsNaN[L]]:
+ return BoundIsNaN[L]
-@dataclass(frozen=True)
-class NotNaN(UnaryPredicate[T]):
- as_bound = BoundNotNaN
- def __invert__(self) -> IsNaN[T]:
+class NotNaN(UnaryPredicate):
+ def __invert__(self) -> IsNaN:
return IsNaN(self.term)
+ @property
+ def as_bound(self) -> Type[BoundNotNaN[L]]:
+ return BoundNotNaN[L]
Review Comment:
I like the suggestion, but unfortunately since `L` can be multiple types,
this can't be done:
```
➜ python git:(fd-remove-dataclass) ✗ pre-commit run --all-files mypy
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1
python/pyiceberg/expressions/__init__.py:419: error: Incompatible return
value type (got "Type[BoundNotNaN[Any]]", expected "Type[BoundNotNaN[str]]")
python/pyiceberg/expressions/__init__.py:419: error: Incompatible return
value type (got "Type[BoundNotNaN[Any]]", expected "Type[BoundNotNaN[bool]]")
python/pyiceberg/expressions/__init__.py:419: error: Incompatible return
value type (got "Type[BoundNotNaN[Any]]", expected "Type[BoundNotNaN[int]]")
python/pyiceberg/expressions/__init__.py:419: error: Incompatible return
value type (got "Type[BoundNotNaN[Any]]", expected "Type[BoundNotNaN[bytes]]")
python/pyiceberg/expressions/__init__.py:419: error: Incompatible return
value type (got "Type[BoundNotNaN[Any]]", expected "Type[BoundNotNaN[UUID]]")
python/pyiceberg/expressions/__init__.py:419: error: Incompatible return
value type (got "Type[BoundNotNaN[Any]]", expected "Type[BoundNotNaN[Decimal]]")
Found 6 errors in 1 file (checked 83 source files)
```
--
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]
