[data] feat: show more detail info for dataset.explain#57798
[data] feat: show more detail info for dataset.explain#57798my-vegetable-has-exploded wants to merge 6 commits intoray-project:masterfrom
Conversation
Signed-off-by: my-vegetable-has-exploded <wy1109468038@gmail.com>
Signed-off-by: my-vegetable-has-exploded <wy1109468038@gmail.com>
Signed-off-by: my-vegetable-has-exploded <wy1109468038@gmail.com>
Signed-off-by: my-vegetable-has-exploded <wy1109468038@gmail.com>
Signed-off-by: my-vegetable-has-exploded <wy1109468038@gmail.com>
764cc78 to
bd83726
Compare
Signed-off-by: my-vegetable-has-exploded <wy1109468038@gmail.com>
|
But currently I don't have a good initial example for customized operator metric (Ray Data invokes numerous third-party libraries, which abstract away many low-level metric details). Do you have any suggestions? @richardliaw |
| raise ValueError(f"Unexpected operator type: {type(op)}") | ||
|
|
||
| prefix = "" if depth == 0 else " " * ((depth - 1) * 3) + "+- " | ||
| curr_str += textwrap.indent(op_str + "\n", prefix) |
There was a problem hiding this comment.
Bug: Incorrect Indentation in Operator Explanations
The textwrap.indent() call in generate_plan_string applies the tree-structure prefix to every line of multi-line operator explanations. This incorrectly indents all continuation lines with the prefix, breaking the physical plan's formatting. The prefix should only apply to the first line of an operator's explanation.
| self._per_block_limit = per_block_limit | ||
|
|
||
| def explain(self, mode: str = "default") -> str: | ||
| return f"{self._name}" |
There was a problem hiding this comment.
Bug: Inconsistent Default Mode in Explain Methods
The AbstractMap.explain() method defaults its mode parameter to "default". This is inconsistent with other explain methods throughout the codebase, which use "simple" as their default, potentially causing unexpected behavior.
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
|
Hey @my-vegetable-has-exploded, thank you for the contribution!
Can you elaborate on what you mean by this? |
Description
A new explain method has been added to the PhysicalOperator class, allowing different PhysicalOperators to customize their explain by overriding it. Since operators like Source and Filter are converted into MapTransformFns within a MapOperator—which may also be fused, so we provide detailed explain function for all MapTransformFns. To facilitate this, MapTransformFn now supports a custom explain_fn that can be passed in and invoked. This PR prototypes this functionality with Range and Parquet sources as initial examples.
Related issues
Part of #55052
Types of change
Checklist
Does this PR introduce breaking changes?
Testing:
Code Quality:
git commit -s)Documentation:
doc/source/(if applicable)Additional context