[GitHub] spark pull request #20788: [WIP][SPARK-21030][PYTHON][SQL] Adds more types f...

2018-03-10 Thread DylanGuedes
Github user DylanGuedes commented on a diff in the pull request:

https://github.com/apache/spark/pull/20788#discussion_r173623998
  
--- Diff: python/pyspark/sql/dataframe.py ---
@@ -437,10 +437,11 @@ def hint(self, name, *parameters):
 if not isinstance(name, str):
 raise TypeError("name should be provided as str, got 
{0}".format(type(name)))
 
+allowed = [str, list, float, int]
 for p in parameters:
-if not isinstance(p, str):
+if not type(p) in allowed:
--- End diff --

Didn't know that it was possible, nice!


---

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



[GitHub] spark pull request #20788: [WIP][SPARK-21030][PYTHON][SQL] Adds more types f...

2018-03-10 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/20788#discussion_r173619046
  
--- Diff: python/pyspark/sql/dataframe.py ---
@@ -437,10 +437,11 @@ def hint(self, name, *parameters):
 if not isinstance(name, str):
 raise TypeError("name should be provided as str, got 
{0}".format(type(name)))
 
+allowed = [str, list, float, int]
 for p in parameters:
-if not isinstance(p, str):
+if not type(p) in allowed:
--- End diff --

Hm? can't we just simply do `isinstance(p, (basestring, list, float, int))`?


---

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



[GitHub] spark pull request #20788: [WIP][SPARK-21030][PYTHON][SQL] Adds more types f...

2018-03-10 Thread DylanGuedes
Github user DylanGuedes commented on a diff in the pull request:

https://github.com/apache/spark/pull/20788#discussion_r173618711
  
--- Diff: python/pyspark/sql/dataframe.py ---
@@ -437,10 +437,11 @@ def hint(self, name, *parameters):
 if not isinstance(name, str):
 raise TypeError("name should be provided as str, got 
{0}".format(type(name)))
 
+allowed = [str, list, float, int]
 for p in parameters:
-if not isinstance(p, str):
+if not type(p) in allowed:
--- End diff --

Oh good to know, good catch. But then should I replicate `isinstance` for 
the other types (int, float, etc)? Or adding unicode to `allowed` is also a 
solution?


---

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



[GitHub] spark pull request #20788: [WIP][SPARK-21030][PYTHON][SQL] Adds more types f...

2018-03-09 Thread felixcheung
Github user felixcheung commented on a diff in the pull request:

https://github.com/apache/spark/pull/20788#discussion_r173612651
  
--- Diff: python/pyspark/sql/dataframe.py ---
@@ -437,10 +437,11 @@ def hint(self, name, *parameters):
 if not isinstance(name, str):
 raise TypeError("name should be provided as str, got 
{0}".format(type(name)))
 
+allowed = [str, list, float, int]
 for p in parameters:
-if not isinstance(p, str):
+if not type(p) in allowed:
--- End diff --

I think isinstance is preferred, string can be str or unicode etc..


---

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



[GitHub] spark pull request #20788: [WIP][SPARK-21030][PYTHON][SQL] Adds more types f...

2018-03-09 Thread DylanGuedes
GitHub user DylanGuedes opened a pull request:

https://github.com/apache/spark/pull/20788

[WIP][SPARK-21030][PYTHON][SQL] Adds more types for hint in pyspark

Signed-off-by: DylanGuedes 

## What changes were proposed in this pull request?

Addition of float, int and list hints for `pyspark.sql` Hint.

## How was this patch tested?

I don't figure out a way to properly test it, and I don't know if this is a 
big problem since it's all transformed in the scala version, that looks well 
tested. 


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/DylanGuedes/spark jira-21030

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/20788.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #20788


commit 4243f1ab8d049b3d9bc8860d591617e5d393bf67
Author: DylanGuedes 
Date:   2018-03-08T21:56:17Z

Adds more types for hint in pyspark

Signed-off-by: DylanGuedes 




---

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