You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/04/10 03:14:28 UTC

[GitHub] [spark] Yikun opened a new pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Yikun opened a new pull request #32118:
URL: https://github.com/apache/spark/pull/32118


   ### What changes were proposed in this pull request?
   
   This patch adds the missing type hints of pyspark.sql.*:
   - pyspark.sql.column._to_java_column
   - pyspark.sql.column._to_seq
   - pyspark.sql.column._create_column_from_literal
   - pyspark.sql.types._parse_datatype_string
   
   ### Why are the changes needed?
   
   To cleanup these error: 
   python/pyspark/pandas/frame.py:6486: error: Module 'pyspark.sql.types' has no attribute '_parse_datatype_string'
   python/pyspark/pandas/spark/functions.py:22: error: Module 'pyspark.sql.column' has no attribute '_to_java_column'
   python/pyspark/pandas/spark/functions.py:22: error: Module 'pyspark.sql.column' has no attribute '_to_seq'
   python/pyspark/pandas/spark/functions.py:22: error: Module 'pyspark.sql.column' has no attribute '_create_column_from_literal'
   
   Related: https://issues.apache.org/jira/browse/SPARK-34941
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   ### How was this patch tested?
   1. Disable the ignore_error on mypy.ini and run mypy
   https://github.com/apache/spark/blob/ee7bf7d9628384548d09ac20e30992dd705c20f4/python/mypy.ini#L132
   2. lint-python passed.
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun commented on a change in pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #32118:
URL: https://github.com/apache/spark/pull/32118#discussion_r610988580



##########
File path: python/pyspark/sql/column.pyi
##########
@@ -116,3 +123,11 @@ class Column:
     def __nonzero__(self) -> None: ...
     def __bool__(self) -> None: ...
     def contains(self, item: Any) -> Column: ...
+
+def _to_java_column(col: ColumnOrName) -> JavaObject: ...

Review comment:
       All missing type hints were [internal use](https://pep8.org/#descriptive-naming-styles) methods, but it still be used by other modules: https://github.com/apache/spark/blob/364d1eaf10f51c357f507325557fb076140ced2c/python/pyspark/pandas/spark/functions.py#L22
   
   I think we should move these methods into pyspark.sql.utils or somewhere in future.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun edited a comment on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun edited a comment on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817128098


   > Could you explain what is the use case for adding these?
   
   The annotating missing warning was found when I cleaunup the mypy error of spark pandas, see more in [SPARK-34941](https://issues.apache.org/jira/browse/SPARK-34941).
   
   [python/pyspark/pandas/frame.py:6486](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/frame.py#L6486): error: Module 'pyspark.sql.types' has no attribute '_parse_datatype_string'
   [python/pyspark/pandas/spark/functions.py:22](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/spark/functions.py#L22): error: Module 'pyspark.sql.column' has no attribute '_to_java_column'
   [python/pyspark/pandas/spark/functions.py:22](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/spark/functions.py#L22): error: Module 'pyspark.sql.column' has no attribute '_to_seq'
   [python/pyspark/pandas/spark/functions.py:22](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/spark/functions.py#L22): error: Module 'pyspark.sql.column' has no attribute
   
   > In general we decided to avoid annotating private and internal code 
   
   Yep, I also found this rule, so I mentioned in previous comment: https://github.com/apache/spark/pull/32118#discussion_r610988580


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun closed pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun closed pull request #32118:
URL: https://github.com/apache/spark/pull/32118


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun commented on a change in pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #32118:
URL: https://github.com/apache/spark/pull/32118#discussion_r610988580



##########
File path: python/pyspark/sql/column.pyi
##########
@@ -116,3 +123,11 @@ class Column:
     def __nonzero__(self) -> None: ...
     def __bool__(self) -> None: ...
     def contains(self, item: Any) -> Column: ...
+
+def _to_java_column(col: ColumnOrName) -> JavaObject: ...

Review comment:
       Some note for reviewer: all missing type hints were [internal use](https://pep8.org/#descriptive-naming-styles) methods as usual, but it still be used by other modules: https://github.com/apache/spark/blob/364d1eaf10f51c357f507325557fb076140ced2c/python/pyspark/pandas/spark/functions.py#L22
   
   Looks like we should move these methods into pyspark.sql.utils or somewhere in future.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zero323 commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817329901


   Thank you for your response. I think we need to discuss this further.  Personally, I'd lean towards using type ignore within newly added pandas modules. We could also switch to stub files to make Pandas consistent with the rest of the code.
   
   Any thoughts, @HyukjinKwon?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] zero323 commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817105893


   Thank you for your proposal @Yikun.
   
   In general we decided to avoid annotating private and internal code (in fact, we even dropped annotations for some modules during original migration).
   
   Could you explain what is the use case for adding these?


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817075146






-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817070123


   **[Test build #137160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137160/testReport)** for PR 32118 at commit [`c740517`](https://github.com/apache/spark/commit/c7405178a0b1db3eb6981c25903194db44c03e36).


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817075145






-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817437454


   @zero323 @HyukjinKwon OK, thanks, let's close it first, I will help on when it's ready to start.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817128098


   > Could you explain what is the use case for adding these?
   
   The annotating missing warning was found when I cleaunup the mypy error of spark pandas, see more in [SPARK-34941](https://issues.apache.org/jira/browse/SPARK-34941).
   
   [python/pyspark/pandas/frame.py:6486](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/frame.py#L6486): error: Module 'pyspark.sql.types' has no attribute '_parse_datatype_string'
   [python/pyspark/pandas/spark/functions.py:22](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/spark/functions.py#L22): error: Module 'pyspark.sql.column' has no attribute '_to_java_column'
   [python/pyspark/pandas/spark/functions.py:22](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/spark/functions.py#L22): error: Module 'pyspark.sql.column' has no attribute '_to_seq'
   [python/pyspark/pandas/spark/functions.py:22](https://github.com/apache/spark/blob/214a46aa88cd682874584dc407ad130a30761884/python/pyspark/pandas/spark/functions.py#L22): error: Module 'pyspark.sql.column' has no attribute


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817435261


   cc @ueshin, @itholic, @xinrong-databricks FYI


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun commented on a change in pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun commented on a change in pull request #32118:
URL: https://github.com/apache/spark/pull/32118#discussion_r610988580



##########
File path: python/pyspark/sql/column.pyi
##########
@@ -116,3 +123,11 @@ class Column:
     def __nonzero__(self) -> None: ...
     def __bool__(self) -> None: ...
     def contains(self, item: Any) -> Column: ...
+
+def _to_java_column(col: ColumnOrName) -> JavaObject: ...

Review comment:
       Some note for reviewer: all missing type hints were [internal use](https://pep8.org/#descriptive-naming-styles) methods, but it still be used by other modules: https://github.com/apache/spark/blob/364d1eaf10f51c357f507325557fb076140ced2c/python/pyspark/pandas/spark/functions.py#L22
   
   I think we should move these methods into pyspark.sql.utils or somewhere in future.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817072086


   **[Test build #137160 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137160/testReport)** for PR 32118 at commit [`c740517`](https://github.com/apache/spark/commit/c7405178a0b1db3eb6981c25903194db44c03e36).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817070123


   **[Test build #137160 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/137160/testReport)** for PR 32118 at commit [`c740517`](https://github.com/apache/spark/commit/c7405178a0b1db3eb6981c25903194db44c03e36).


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] Yikun commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
Yikun commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817070578


   cc @zero323


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817073200


   Kubernetes integration test unable to build dist.
   
   exiting with code: 1
   URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/41739/
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #32118: [SPARK-35015][PYTHON] Add missing type hints of pyspark.sql.*

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32118:
URL: https://github.com/apache/spark/pull/32118#issuecomment-817434741


   Yeah, I would prefer to ignore them for now too. It's planned to be revisited at SPARK-34941.


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org