Skip to content

Make UnaryPredicate JSON Serializable and Add Unit Tests - #2598

Merged
Fokko merged 6 commits into
apache:mainfrom
Aniketsy:make-unarypredicate-json-serializable
Oct 22, 2025
Merged

Make UnaryPredicate JSON Serializable and Add Unit Tests#2598
Fokko merged 6 commits into
apache:mainfrom
Aniketsy:make-unarypredicate-json-serializable

Conversation

@Aniketsy

Copy link
Copy Markdown
Contributor

#2522

This PR makes the UnaryPredicate class and its subclasses (IsNull, NotNull, IsNaN, NotNaN) JSON serializable using Pydantic

  • Adds unit tests to verify JSON serialization for IsNull and NotNull

Please let me know if my approach or fix needs any improvements . I’m open to feedback and happy to make changes based on suggestions.
Thankyou !

@Aniketsy
Aniketsyforce-pushed the make-unarypredicate-json-serializable branch from ad99ec8 to 9d333eeCompareOctober 18, 2025 08:28
@Aniketsy

Copy link
Copy Markdown
ContributorAuthor

@Fokko please review these changes , when you get a chance.

Comment threadpyiceberg/expressions/__init__.py Outdated
Comment threadpyiceberg/expressions/__init__.py Outdated
@Aniketsy
Aniketsyforce-pushed the make-unarypredicate-json-serializable branch from 4096627 to 9c9b3c4CompareOctober 20, 2025 08:50
Comment threadpyiceberg/expressions/__init__.py
Co-authored-by: Fokko Driesprong <fokko@apache.org>
@Aniketsy

Copy link
Copy Markdown
ContributorAuthor

@Fokko While testing locally, I’m getting 15 test failures, but in the CI check, there’s only 1 test failing. Could you please help me understand what might be causing this difference?

@Fokko

Copy link
Copy Markdown
Contributor

@Aniketsy Sure thing, I'm happy to. Could you share the output of the console?

Comment threadtests/expressions/test_expressions.py Outdated
@Aniketsy

Copy link
Copy Markdown
ContributorAuthor

sure,

=================== short test summary info ===================
FAILED tests/expressions/test_expressions.py::test_reference - AttributeError: 'Reference' object has no attribute 'model_... FAILED tests/expressions/test_expressions.py::test_always_true - AttributeError: 'AlwaysTrue' object has no attribute 'model... FAILED tests/expressions/test_expressions.py::test_always_false - AttributeError: 'AlwaysFalse' object has no attribute 'mode...FAILED tests/expressions/test_expressions.py::test_serialize_is_null - AttributeError: 'IsNull' object has no attribute 'model_dum...
FAILED tests/expressions/test_expressions.py::test_serialize_not_null - AttributeError: 'NotNull' object has no attribute 'model_du...
FAILED tests/expressions/test_expressions.py::test_serialize_in - AttributeError: 'In' object has no attribute 'model_dump_json'FAILED tests/expressions/test_expressions.py::test_serialize_not_in - AttributeError: 'NotIn' object has no attribute 'model_dump...
FAILED tests/expressions/test_expressions.py::test_equal_to - AttributeError: 'EqualTo' object has no attribute 'model_du... FAILED tests/expressions/test_expressions.py::test_not_equal_to - AttributeError: 'NotEqualTo' object has no attribute 'model...FAILED tests/expressions/test_expressions.py::test_greater_than_or_equal_to - AttributeError: 'GreaterThanOrEqual' object has no attribut...
FAILED tests/expressions/test_expressions.py::test_greater_than - AttributeError: 'GreaterThan' object has no attribute 'mode...FAILED tests/expressions/test_expressions.py::test_less_than - AttributeError: 'LessThan' object has no attribute 'model_d... FAILED tests/expressions/test_expressions.py::test_less_than_or_equal - AttributeError: 'LessThanOrEqual' object has no attribute '...
FAILED tests/expressions/test_expressions.py::test_starts_with - AttributeError: 'StartsWith' object has no attribute 'model... FAILED tests/expressions/test_expressions.py::test_not_starts_with - AttributeError: 'NotStartsWith' object has no attribute 'mo...
=============== 15 failed, 145 passed in 1.65s 

Co-authored-by: Fokko Driesprong <fokko@apache.org>

@FokkoFokko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one @Aniketsy! This looks good to me 👍

@Fokko
Fokko merged commit 924ee73 into apache:mainOct 22, 2025
8 checks passed
@Aniketsy

Copy link
Copy Markdown
ContributorAuthor

@Fokko Thanks!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@Aniketsy@Fokko