rdblue commented on code in PR #6139: URL: https://github.com/apache/iceberg/pull/6139#discussion_r1022080677
########## python/pyiceberg/expressions/__init__.py: ########## @@ -321,209 +398,271 @@ def __invert__(self) -> BoundIsNaN[T]: 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[T]]: + return BoundIsNaN[T] -@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[T]]: + return BoundNotNaN[T] + -@dataclass(frozen=True) -class SetPredicate(UnboundPredicate[T]): - literals: tuple[Literal[T], ...] +class SetPredicate(Generic[T], UnboundPredicate, ABC): + literals: Set[Literal[T]] - def __invert__(self) -> SetPredicate[T]: - """Inverted expression of the SetPredicate""" - raise NotImplementedError + def __init__(self, term: Union[str, UnboundTerm], literals: Union[Iterable[Any], Iterable[Literal[T]]]): + super().__init__(term) + self.literals = _convert_into_set(literals) - def bind(self, schema: Schema, case_sensitive: bool = True) -> BooleanExpression: + def bind(self, schema: Schema, case_sensitive: bool = True) -> BoundSetPredicate: bound_term = self.term.bind(schema, case_sensitive) return self.as_bound(bound_term, {lit.to(bound_term.ref().field.field_type) for lit in self.literals}) + def __str__(self): + # Sort to make it deterministic + return f"{str(self.__class__.__name__)}({str(self.term)}, {{{', '.join(sorted([str(literal) for literal in self.literals]))}}})" + + def __repr__(self) -> str: + # Sort to make it deterministic + return f"{str(self.__class__.__name__)}({repr(self.term)}, {{{', '.join(sorted([repr(literal) for literal in self.literals]))}}})" + + def __eq__(self, other: Any) -> bool: + return self.term == other.term and self.literals == other.literals if isinstance(other, SetPredicate) else False + + @property + @abstractmethod + def as_bound(self) -> Type[BoundSetPredicate[T]]: + return BoundSetPredicate[T] + -@dataclass(frozen=True) -class BoundSetPredicate(BoundPredicate[T]): - literals: set[Literal[T]] +class BoundSetPredicate(BoundPredicate[T], ABC): + literals: Set[Literal[T]] - def __invert__(self) -> BoundSetPredicate[T]: - """Inverted expression of the SetPredicate""" - raise NotImplementedError + def __init__(self, term: BoundTerm[T], literals: Union[Iterable[Any], Iterable[Literal[T]]]): + super().__init__(term) + self.literals = _convert_into_set(literals) # pylint: disable=W0621 + + def __str__(self): + # Sort to make it deterministic + return f"{str(self.__class__.__name__)}({str(self.term)}, {{{', '.join(sorted([str(literal) for literal in self.literals]))}}})" + + def __repr__(self) -> str: + # Sort to make it deterministic + return f"{str(self.__class__.__name__)}({repr(self.term)}, {{{', '.join(sorted([repr(literal) for literal in self.literals]))}}})" + + def __eq__(self, other: Any) -> bool: + return self.term == other.term and self.literals == other.literals if isinstance(other, BoundSetPredicate) else False -@dataclass(frozen=True) class BoundIn(BoundSetPredicate[T]): - def __new__(cls, term: BoundTerm[T], literals: set[Literal[T]]): # pylint: disable=W0221 - count = len(literals) + def __new__(cls, term: BoundTerm[T], literals: Union[Iterable[Any], Iterable[Literal[T]]]): # pylint: disable=W0221 Review Comment: Since this is only intended to be called inside a `bind` method, can we skip accepting `Iterable[Any]` and be strict that the incoming type should be `Iterable[Literal[T]]`? That helps keep bound expression type safety. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org