[SPARK-37421][PYTHON] Inline type hints for python/pyspark/mllib/evaluation.py #34680

dchvn · 2021-11-22T07:09:51Z

What changes were proposed in this pull request?

Inline type hints for evaluation.py in python/pyspark/mllib/

Why are the changes needed?

We can 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 tests

dchvn · 2021-11-22T07:17:32Z

cc @zero323, @HyukjinKwon, @ueshin FYI. Many thanks!

dchvn · 2021-11-22T07:21:09Z

python/pyspark/mllib/evaluation.py

        java_model = java_class(df._jdf)
        super(BinaryClassificationMetrics, self).__init__(java_model)

-    @property
+    @property  # type: ignore[misc]


I met error: Decorated property not supported [misc] when checking mypy for every @property. As mypy's issue #1362, I think we should ignore this.

SparkQA · 2021-11-22T08:16:54Z

Test build #145503 has finished for PR 34680 at commit e686682.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
java_class = (
java_class = (
java_class = (
java_class = (

SparkQA · 2021-11-22T08:50:21Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49975/

SparkQA · 2021-11-22T09:32:49Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49975/

zero323 · 2021-11-22T10:53:20Z

Let's wait with this until we clear the backlog and all dependencies (to clearly handle this, and majority of modules from ml and mllib, we need SPARK-37094 to be completed) are annotated.

If there are any modules that have no direct dependencies on core, that's not an issue (though it seems like we already saturated our reviewing capabilities for this particular component), but I think we want to avoid having FOLLOW-UPs for each module, or worse, putting more strain on SPARK-37152

…uation.py

zero323 · 2022-02-21T10:33:42Z

Both RDD and core mllib modules should have stable annotations for the moment, so it might be a good idea to resync this and other PRs and check if everything still passes (and if any quotes, casts, and type ignores can be removed).

dchvn · 2022-02-21T10:55:22Z

Can you have a look back in #37405 before moving to mllib? @zero323 , many thanks!

dchvn · 2022-02-22T06:33:13Z

cc @zero323 , I have updated this PR, hope you review it again. Thanks

dchvn · 2022-02-22T23:52:50Z

cc @HyukjinKwon , TIA

zero323

Didn't check this thoroughly, but at first glance looks OK.

We might change

spark/python/pyspark/mllib/evaluation.py

Line 412 in 0543d80

class RankingMetrics(JavaModelWrapper):

to

class RankingMetrics(JavaModelWrapper, Generic[T]):

though it doesn't make much difference, as far as I am aware.

dchvn · 2022-02-24T03:17:08Z

Didn't check this thoroughly, but at first glance looks OK.

We might change

spark/python/pyspark/mllib/evaluation.py

Line 412 in 0543d80

class RankingMetrics(JavaModelWrapper):

to
class RankingMetrics(JavaModelWrapper, Generic[T]): 
though it doesn't make much difference, as far as I am aware.

updated

dchvn · 2022-02-24T03:30:59Z

@zero323 Could you please review the latest update to me as soon as possible?
many thanks

zero323 · 2022-02-26T14:26:43Z

Sorry @dchvn, but I won't be available for reviews at the moment. Given the situation, my properties are somewhere else (blame Putin).

@itholic, @ueshin, @xinrong-databricks ‒ would be so kind and take a look at the and remaining ML / MLLib tasks? Important prerequisites are already done, so the remaining PRs should go smoothly. I'll owe you one!

dchvn · 2022-02-26T15:10:10Z

Sorry @dchvn, but I won't be available for reviews at the moment. Given the situation, my properties are somewhere else (blame Putin).

@itholic, @ueshin, @xinrong-databricks ‒ would be so kind and take a look at the and remaining ML / MLLib tasks? Important prerequisites are already done, so the remaining PRs should go smoothly. I'll owe you one!

Hope you are not affected by the war, good luck!

zero323 · 2022-02-27T12:28:27Z

Hope you are not affected by the war, good luck!

Oh, I am on the safe side of the border, so I am OK. There is just tons of stuff to be done here, will all the people coming.

itholic

Looks fine to me.

Btw, maybe we can create the ticket to track the https://github.com/apache/spark/pull/34680/files#r754003277 so that we can resolve the current ignores after mypy issue resolving ??

zero323 · 2022-03-06T12:59:21Z

Btw, maybe we can create the ticket to track the https://github.com/apache/spark/pull/34680/files#r754003277 so that we can resolve the current ignores after mypy issue resolving ??

@itholic I was actually thinking about disallowing dead casts and ignores completely #35740. This should handle properties as well, in case it is solved upstream.

zero323 · 2022-03-06T13:08:00Z

python/pyspark/mllib/evaluation.py

@@ -61,7 +67,7 @@ class BinaryClassificationMetrics(JavaModelWrapper):
    0.88...
    """

-    def __init__(self, scoreAndLabels):
+    def __init__(self, scoreAndLabels: RDD[Tuple[float, float]]) -> None:


Nit. No need for -> None here.

updated! Thanks

zero323

I made a quick pass and it looks good, as long as #34680 (review) is addressed.

itholic

Looks pretty good except #34680 (comment)

itholic · 2022-03-09T09:34:34Z

LGTM. cc @zero323

zero323 · 2022-03-09T10:16:32Z

Merged into master.

dchvn · 2022-03-09T10:26:00Z

Thanks all! Hope you review all my PR in SPARK-37396 and SPARK-37395

…uation.py ### What changes were proposed in this pull request? Inline type hints for evaluation.py in python/pyspark/mllib/ ### Why are the changes needed? We can 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 tests Closes apache#34680 from dchvn/SPARK-37421. Lead-authored-by: dch nguyen <dchvn.dgd@gmail.com> Co-authored-by: dch nguyen <dgd_contributor@viettel.com.vn> Signed-off-by: zero323 <mszymkiewicz@gmail.com>

github-actions bot added CORE MLLIB PYTHON labels Nov 22, 2021

dchvn commented Nov 22, 2021

View reviewed changes

This was referenced Nov 22, 2021

[SPARK-37407][PYTHON] Inline type hints for python/pyspark/ml/functions.py #34678

Closed

[SPARK-37399][SPARK-37403][PySpark][ML] Merge {ml, mllib}/common.pyi into common.py #34671

Closed

dchvn and others added 2 commits December 29, 2021 11:11

[SPARK-37421][PYTHON] Inline type hints for python/pyspark/mllib/eval…

076093d

…uation.py

update code

ebc5040

dchvn force-pushed the SPARK-37421 branch from e686682 to ebc5040 Compare December 30, 2021 01:46

reformat python

a0d582d

dchvn added 2 commits February 22, 2022 18:06

Merge branch 'master' into SPARK-37421

712fc21

update

0543d80

zero323 reviewed Feb 23, 2022

View reviewed changes

dchvn requested a review from zero323 February 24, 2022 03:16

dchvn added 2 commits February 24, 2022 16:25

Merge branch 'master' into SPARK-37421

a4d4662

update20220224

0bacc38

itholic reviewed Mar 3, 2022

View reviewed changes

zero323 reviewed Mar 6, 2022

View reviewed changes

itholic approved these changes Mar 7, 2022

View reviewed changes

dchvn requested a review from zero323 March 8, 2022 06:40

dchvn added 2 commits March 8, 2022 20:38

Merge branch 'master' into SPARK-37421

7f80920

remove -> None

794c44a

zero323 closed this in 62e4c29 Mar 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-37421][PYTHON] Inline type hints for python/pyspark/mllib/evaluation.py #34680

[SPARK-37421][PYTHON] Inline type hints for python/pyspark/mllib/evaluation.py #34680

dchvn commented Nov 22, 2021

dchvn commented Nov 22, 2021

dchvn Nov 22, 2021

SparkQA commented Nov 22, 2021

SparkQA commented Nov 22, 2021

SparkQA commented Nov 22, 2021

zero323 commented Nov 22, 2021 •

edited

Loading

zero323 commented Feb 21, 2022

dchvn commented Feb 21, 2022

dchvn commented Feb 22, 2022

dchvn commented Feb 22, 2022

zero323 left a comment

dchvn commented Feb 24, 2022

dchvn commented Feb 24, 2022

zero323 commented Feb 26, 2022

dchvn commented Feb 26, 2022

zero323 commented Feb 27, 2022

itholic left a comment •

edited

Loading

zero323 commented Mar 6, 2022

zero323 Mar 6, 2022

dchvn Mar 8, 2022

zero323 left a comment

itholic left a comment

itholic commented Mar 9, 2022

zero323 commented Mar 9, 2022

dchvn commented Mar 9, 2022

[SPARK-37421][PYTHON] Inline type hints for python/pyspark/mllib/evaluation.py #34680

[SPARK-37421][PYTHON] Inline type hints for python/pyspark/mllib/evaluation.py #34680

Conversation

dchvn commented Nov 22, 2021

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

dchvn commented Nov 22, 2021

dchvn Nov 22, 2021

Choose a reason for hiding this comment

SparkQA commented Nov 22, 2021

SparkQA commented Nov 22, 2021

SparkQA commented Nov 22, 2021

zero323 commented Nov 22, 2021 • edited Loading

zero323 commented Feb 21, 2022

dchvn commented Feb 21, 2022

dchvn commented Feb 22, 2022

dchvn commented Feb 22, 2022

zero323 left a comment

Choose a reason for hiding this comment

dchvn commented Feb 24, 2022

dchvn commented Feb 24, 2022

zero323 commented Feb 26, 2022

dchvn commented Feb 26, 2022

zero323 commented Feb 27, 2022

itholic left a comment • edited Loading

Choose a reason for hiding this comment

zero323 commented Mar 6, 2022

zero323 Mar 6, 2022

Choose a reason for hiding this comment

dchvn Mar 8, 2022

Choose a reason for hiding this comment

zero323 left a comment

Choose a reason for hiding this comment

itholic left a comment

Choose a reason for hiding this comment

itholic commented Mar 9, 2022

zero323 commented Mar 9, 2022

dchvn commented Mar 9, 2022

zero323 commented Nov 22, 2021 •

edited

Loading

itholic left a comment •

edited

Loading