Fokko commented on code in PR #2561:
URL: https://github.com/apache/iceberg-python/pull/2561#discussion_r2424877615
##########
pyiceberg/expressions/__init__.py:
##########
@@ -743,12 +749,33 @@ def as_bound(self) -> Type[BoundNotIn[L]]:
return BoundNotIn[L]
-class LiteralPredicate(UnboundPredicate[L], ABC):
- literal: Literal[L]
+class LiteralPredicate(IcebergBaseModel, UnboundPredicate[L], ABC):
+ type: TypingLiteral["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq",
"starts-with", "not-starts-with"] = Field(alias="type")
+ term: UnboundTerm[L]
+ literal: Literal[L] = Field(serialization_alias="value")
Review Comment:
```suggestion
value: Literal[L] = Field()
```
##########
pyiceberg/expressions/__init__.py:
##########
@@ -743,12 +749,33 @@ def as_bound(self) -> Type[BoundNotIn[L]]:
return BoundNotIn[L]
-class LiteralPredicate(UnboundPredicate[L], ABC):
- literal: Literal[L]
+class LiteralPredicate(IcebergBaseModel, UnboundPredicate[L], ABC):
+ type: TypingLiteral["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq",
"starts-with", "not-starts-with"] = Field(alias="type")
+ term: UnboundTerm[L]
+ literal: Literal[L] = Field(serialization_alias="value")
+
+ model_config = ConfigDict(arbitrary_types_allowed=True)
+
+ def __init__(self, *args: Any, **kwargs: Any) -> None:
+ if args:
+ if len(args) != 2:
+ raise TypeError("Expected (term, literal)")
+ kwargs = {"term": args[0], "literal": args[1], **kwargs}
+ super().__init__(**kwargs)
+
+ @field_validator("term", mode="before")
+ @classmethod
+ def _coerce_term(cls, v: Any) -> UnboundTerm[Any]:
+ return _to_unbound_term(v)
- def __init__(self, term: Union[str, UnboundTerm[Any]], literal: Union[L,
Literal[L]]): # pylint: disable=W0621
- super().__init__(term)
- self.literal = _to_literal(literal) # pylint: disable=W0621
+ @field_validator("literal", mode="before")
+ @classmethod
+ def _coerce_literal(cls, v: Union[L, Literal[L]]) -> Literal[L]:
+ return _to_literal(v)
+
+ @field_serializer("literal")
+ def ser_literal(self, literal: Literal[L]) -> str:
+ return "Any"
Review Comment:
If we just call the field `value`, and we add a `literal` property:
```python
@property
def literal(self) -> Literal[L]:
return self.value
```
##########
pyiceberg/expressions/__init__.py:
##########
@@ -743,12 +749,33 @@ def as_bound(self) -> Type[BoundNotIn[L]]:
return BoundNotIn[L]
-class LiteralPredicate(UnboundPredicate[L], ABC):
- literal: Literal[L]
+class LiteralPredicate(IcebergBaseModel, UnboundPredicate[L], ABC):
+ type: TypingLiteral["lt", "lt-eq", "gt", "gt-eq", "eq", "not-eq",
"starts-with", "not-starts-with"] = Field(alias="type")
+ term: UnboundTerm[L]
+ literal: Literal[L] = Field(serialization_alias="value")
+
+ model_config = ConfigDict(arbitrary_types_allowed=True)
+
+ def __init__(self, *args: Any, **kwargs: Any) -> None:
+ if args:
+ if len(args) != 2:
+ raise TypeError("Expected (term, literal)")
+ kwargs = {"term": args[0], "literal": args[1], **kwargs}
+ super().__init__(**kwargs)
Review Comment:
Always! So, I think the linter isn't really sure what to do. It is pretty
clear:
In the signature, we see that `transform` accepts an optional, which I think
is correct. However, `_transform_literal` requires non-null, which is incorrect.
```python
def _truncate_number(
name: str, pred: BoundLiteralPredicate[L], transform:
Callable[[Optional[L]], Optional[L]]
) -> Optional[UnboundPredicate[Any]]:
boundary = pred.literal
if not isinstance(boundary, (LongLiteral, DecimalLiteral, DateLiteral,
TimestampLiteral)):
raise ValueError(f"Expected a numeric literal, got:
{type(boundary)}")
if isinstance(pred, BoundLessThan):
return LessThanOrEqual(Reference(name),
_transform_literal(transform, boundary.decrement()))
elif isinstance(pred, BoundLessThanOrEqual):
```
The following change suppresses most of the warnings for me:
```python
-def _transform_literal(func: Callable[[L], L], lit: Literal[L]) ->
Literal[L]:
+def _transform_literal(func: Callable[[Any], Any], lit: Literal[L]) ->
Literal[L]:
"""Small helper to upwrap the value from the literal, and wrap it
again."""
return literal(func(lit.value))
```
--
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]
