Skip to content

Conversation

@goutamvenkat-anyscale
Copy link
Contributor

@goutamvenkat-anyscale goutamvenkat-anyscale commented Nov 16, 2025

Description

Ensure the predicate expr appears correctly for the Filter logical op.

Related issues

Closes #58620

Additional information

Optional: Add implementation details, API changes, usage examples, screenshots, etc.

@goutamvenkat-anyscale goutamvenkat-anyscale requested a review from a team as a code owner November 16, 2025 07:19
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces an _InlineExprReprVisitor to generate concise string representations for expressions, which improves the readability of logical plans for operators like Filter. The implementation is a good step forward, but I've identified a critical issue regarding operator precedence in the generated strings that could lead to ambiguous or incorrect representations. My review includes suggestions to add parentheses to ensure the correct order of operations is visually preserved. Addressing this will make the feature more robust and reliable.

Signed-off-by: Goutam <[email protected]>
Signed-off-by: Goutam <[email protected]>
Signed-off-by: Goutam <[email protected]>
@goutamvenkat-anyscale
Copy link
Contributor Author

/gemini summary

@goutamvenkat-anyscale goutamvenkat-anyscale added data Ray Data-related issues go add ONLY when ready to merge, run all tests labels Nov 16, 2025
@gemini-code-assist
Copy link
Contributor

Summary of Changes

This pull request significantly improves the debuggability and clarity of Ray Data's logical plans by replacing generic expression placeholders with detailed, inline string representations of the actual expressions. This change allows users to quickly understand the logic applied within operators like filter, making it easier to trace data transformations and identify potential issues.

Highlights

  • Enhanced Operator Naming: Expression-based operators in Ray Data's logical plan now display concise, inline string representations of their underlying expressions, replacing the generic "" placeholder.
  • New Expression Visitor: Introduced _InlineExprReprVisitor to generate human-readable, single-line string representations of various expression types (e.g., column, literal, binary, unary, UDF).
  • Operator Precedence Handling: The new visitor includes logic to correctly handle operator precedence in binary and unary expressions by strategically adding parentheses, ensuring unambiguous representation.
  • Improved Test Coverage: Added new tests to verify the correct display of expression names in filter operations and to comprehensively test the inline representation of complex expressions.
Changelog
  • python/ray/data/_internal/logical/operators/map_operator.py
    • Updated _get_operator_name to utilize the new _InlineExprReprVisitor for generating descriptive names for expression-based operators.
  • python/ray/data/_internal/planner/plan_expression/expression_visitors.py
    • Added the _InlineExprReprVisitor class, which provides methods to convert different expression types into concise string formats, including handling operator symbols and precedence.
  • python/ray/data/tests/test_filter.py
    • Introduced test_filter_expression_display_names to validate that filter operations correctly show their expression names in the plan output.
  • python/ray/data/tests/unit/test_expressions.py
    • Added test_expression_inline_repr to thoroughly test the new inline expression visitor with complex expression structures and truncation.
Activity
  • goutamvenkat-anyscale requested a summary.
  • gemini-code-assist[bot] provided feedback on visit_binary and visit_unary methods, recommending the addition of parentheses to correctly handle operator precedence and avoid ambiguous string representations.

@alexeykudinkin alexeykudinkin merged commit e342775 into ray-project:master Nov 24, 2025
6 checks passed
@goutamvenkat-anyscale goutamvenkat-anyscale deleted the goutam/inline_expr_repr branch November 24, 2025 20:13
ykdojo pushed a commit to ykdojo/ray that referenced this pull request Nov 27, 2025
## Description
Ensure the predicate expr appears correctly for the `Filter` logical op.

## Related issues
Closes ray-project#58620
## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>
Signed-off-by: YK <[email protected]>
SheldonTsen pushed a commit to SheldonTsen/ray that referenced this pull request Dec 1, 2025
## Description
Ensure the predicate expr appears correctly for the `Filter` logical op.

## Related issues
Closes ray-project#58620 
## Additional information
> Optional: Add implementation details, API changes, usage examples,
screenshots, etc.

---------

Signed-off-by: Goutam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

data Ray Data-related issues go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Ray Data] filter with expression UDF shows Filter(NoneType) in the plan

2 participants