spark-test commented on code in PR #59774:
URL: https://github.com/apache/spark/pull/59774#discussion_r4372864343
##########
python/pyspark/eval_handlers/_base.py:
##########
@@ -150,8 +150,8 @@ class EvalTypeHandler(Generic[InputBatch, OutputBatch],
metaclass=_EvalTypeHandl
variant_value: ClassVar[Any] = None
# Whether the handler's UDFs return an iterator.
_iter_based: ClassVar[bool] = False
- # Whether __init__ wraps the UDF in fail_on_stopiteration, so a
StopIteration raised in
- # user code fails the task instead of silently ending the loop that calls
the UDF.
+ # Whether __init__ wraps the UDF in fail_on_stopiteration, so a
StopIteration raised by
Review Comment:
**Nit (P3):** This holds when `spark.sql.pyspark.udf.profiler` is unset, but
not for iterator handlers under a profiler. For `_iter_based` handlers, which
after this PR include `PandasScalarIterUDFHandler` and
`PandasScalarIterElementwiseUDFHandler`, `perf` (and `memory` with
memory_profiler installed) makes `_with_profiler` return a generator function
whose body makes the user call (`iter(f(*args, **kwargs))` / `g = f(*args,
**kwargs)`). `__init__` then wraps that generator function in
`fail_on_stopiteration`, so calling it only creates a generator and the
wrapper's `except StopIteration` never runs. The eager `next()` on empty input
from the new tests then escapes the profiling generator as PEP 479's
`RuntimeError: generator raised StopIteration`, which is the outcome this
comment says the wrap replaces. A standalone mirror of the composition
reproduces both outcomes. Since this is the only in-code description of what
the flag guarantees, a maintainer changing profiling or the w
rap order would be misled. Could the comment say that the conversion applies
to a StopIteration raised synchronously by the call, and that a configured UDF
profiler defers an iterator handler's call into a generator, where the generic
RuntimeError still applies?
**Verification:**
- **Inspection:** The rewritten comment no longer claims
STOP_ITERATION_OCCURRED for profiled iterator handlers. It names the condition
under which the conversion applies, matching the order in which the handler
base applies profiling and fail_on_stopiteration, and it stays within the
100-character line limit with ASCII-only text.
--
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]
