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 2022/12/09 07:02:39 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request, #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

HyukjinKwon opened a new pull request, #38994:
URL: https://github.com/apache/spark/pull/38994

   ### What changes were proposed in this pull request?
   
   This PR proposes to resolve the circular imports workarounds
   
   
   ### Why are the changes needed?
   
   For better readability and maintanence.
   
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, dev-only.
   
   ### How was this patch tested?
   
   CI in this PR should test it out.


-- 
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: reviews-unsubscribe@spark.apache.org

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 #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

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

   oh yeah. the tests passed but the linter failed. I just removed one ignore in linter that's not used, and I checked it locally.
   
   But sorry I should have waited for the test results. Will make sure on that next time. 


-- 
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: reviews-unsubscribe@spark.apache.org

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 #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

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

   Merged to master.


-- 
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: reviews-unsubscribe@spark.apache.org

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] dongjoon-hyun commented on pull request #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38994:
URL: https://github.com/apache/spark/pull/38994#issuecomment-1344945736

   Ur, does this pass CI?


-- 
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: reviews-unsubscribe@spark.apache.org

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 #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

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

   cc @amaliujia 


-- 
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: reviews-unsubscribe@spark.apache.org

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 a diff in pull request #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #38994:
URL: https://github.com/apache/spark/pull/38994#discussion_r1044928308


##########
python/pyspark/sql/connect/column.py:
##########
@@ -706,28 +705,30 @@ def substr(self, startPos: Union[int, "Column"], length: Union[int, "Column"]) -
                     length_t=type(length),
                 )
             )
-        from pyspark.sql.connect.function_builder import functions as F
 
         if isinstance(length, int):
-            length_exp = self._lit(length)
+            length_exp = lit(length)
         elif isinstance(length, Column):
             length_exp = length
         else:
             raise TypeError("Unsupported type for substr().")
 
         if isinstance(startPos, int):
-            start_exp = self._lit(startPos)
+            start_exp = lit(startPos)
         else:
             start_exp = startPos
 
-        return F.substr(self, start_exp, length_exp)
+        return F.substr(self, start_exp, length_exp)  # type: ignore[attr-defined]

Review Comment:
   ```suggestion
           return F.substr(self, start_exp, length_exp)
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

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 #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

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

   I prefer to merge https://github.com/apache/spark/pull/38991 first but please don't bother. I don't mind resolving conflicts 👍 


-- 
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: reviews-unsubscribe@spark.apache.org

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] dongjoon-hyun commented on pull request #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on PR #38994:
URL: https://github.com/apache/spark/pull/38994#issuecomment-1344949254

   No problem at all. If you checked locally, that's more than enough. :)


-- 
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: reviews-unsubscribe@spark.apache.org

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 closed pull request #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect

Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #38994: [SPARK-41329][CONNECT] Resolve circular imports in Spark Connect
URL: https://github.com/apache/spark/pull/38994


-- 
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: reviews-unsubscribe@spark.apache.org

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