[SPARK-37014][PYTHON] Inline type hints for python/pyspark/streaming/context.py#34293
[SPARK-37014][PYTHON] Inline type hints for python/pyspark/streaming/context.py#34293dchvn wants to merge 7 commits intoapache:masterfrom
Conversation
|
Test build #144297 has finished for PR 34293 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
CC @HyukjinKwon @ueshin @zero323 FYI. Thanks! |
|
Test build #144608 has finished for PR 34293 at commit
|
|
CC @HyukjinKwon @ueshin @zero323 FYI. Thanks! Seem jenkins fail because jenkins server is down. |
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
retest this please |
|
Test build #144696 has finished for PR 34293 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
Test build #145101 has finished for PR 34293 at commit
|
|
Kubernetes integration test starting |
|
Kubernetes integration test status failure |
|
cc @itholic mind reviewing this please? |
itholic
left a comment
itholic
left a comment
There was a problem hiding this comment.
Otherwise, looks fine to me
python/pyspark/streaming/context.py
Outdated
There was a problem hiding this comment.
I think we can directly use DStream[str] here.
"DStream[str]" -> DStream[str] should be fine.
There was a problem hiding this comment.
Thanks for you reviewing!
We need to wait to inline type hints python/pyspark/streaming/dstream.py and have DStream(Generic[T_co]) before use DStream[str] directly
There was a problem hiding this comment.
To be precise, for DStream[T] you need DStream to be runtime generic. To avoid further rewrites, we can mark them as generics within this PR.
python/pyspark/streaming/context.py
Outdated
python/pyspark/streaming/context.py
Outdated
python/pyspark/streaming/context.py
Outdated
|
This should be resynced with master. |
|
Seems fine to me. Would you mind taking a last look for this, @zero323 ?? |
It requires re-syncing with master and removal of obsolete ignores, but looks OK otherwise. |
|
@dchvn Would you mind rebasing to the master? |
|
(BTW, there's a bug in GitHub Actions' RESTful API so the check fails. I sent an email to dev mailing list "PR builder not working now") |
itholic
left a comment
itholic
left a comment
There was a problem hiding this comment.
Looks good.
Btw, test is passed in @dchvn 's forked repository: https://github.com/dchvn/spark/actions/runs/2152211443
…context.py ### What changes were proposed in this pull request? Inline type hints for python/pyspark/streaming/context.py from Inline type hints for python/pyspark/streaming/context.pyi. ### Why are the changes needed? Currently, there is type hint stub files python/pyspark/streaming/context.pyi to show the expected types for functions, but we can also take advantage of static type checking within the functions by inlining the type hints. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Existing test. Closes #34293 from dchvn/SPARK-37014. Authored-by: dch nguyen <dchvn.dgd@gmail.com> Signed-off-by: zero323 <mszymkiewicz@gmail.com> (cherry picked from commit c0c1f35) Signed-off-by: zero323 <mszymkiewicz@gmail.com>
|
Thanks for looking into the this @HyukjinKwon and @itholic. LGTM (all tests passed locally). |
|
Merged into master and branch-3.3. |
|
Thank all ❤️ |
5 participants
What changes were proposed in this pull request?
Inline type hints for python/pyspark/streaming/context.py from Inline type hints for python/pyspark/streaming/context.pyi.
Why are the changes needed?
Currently, there is type hint stub files python/pyspark/streaming/context.pyi to show the expected types for functions, but we can also take advantage of static type checking within the functions by inlining the type hints.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Existing test.