[GitHub] [iceberg] Fokko commented on a diff in pull request #6462: Python: Check for nans when creating literal

[GitBox <gi...@apache.org>]

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)
 
 
+@pytest.mark.parametrize("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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org