Fokko commented on code in PR #6462:
URL: https://github.com/apache/iceberg/pull/6462#discussion_r1053565212
##########
python/pyiceberg/expressions/literals.py:
##########
@@ -65,6 +66,9 @@ class Literal(Generic[L], ABC):
def __init__(self, value: L, value_type: Type[L]):
if value is None or not isinstance(value, value_type):
raise TypeError(f"Invalid literal value: {value!r} (not a
{value_type})")
+ if isinstance(value, float):
Review Comment:
Python has lazy evaluation, so we can combine these in the same line 👍🏻
```suggestion
if isinstance(value, float) and isnan(value):
```
##########
python/tests/expressions/test_literals.py:
##########
@@ -89,12 +89,19 @@ def test_literal_from_none_error() -> None:
BinaryLiteral,
],
)
-def test_string_literal_with_none_value_error(literal_class:
Type[PrimitiveType]) -> None:
+def test_literal_classes_with_none_type_error(literal_class:
Type[PrimitiveType]) -> None:
with pytest.raises(TypeError) as e:
literal_class(None)
assert "Invalid literal value: None" in str(e.value)
[email protected]("literal_class", [FloatLiteral, DoubleLiteral])
Review Comment:
Could you add one more tests where we use `literal(float("NaN"))`? The
`literal` is meant as the public API, and people shouldn't necessarily
initialize the specific literal instances themselves.
--
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]
