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/10/28 04:05:16 UTC
[GitHub] [spark] ByronHsu opened a new pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
ByronHsu opened a new pull request #34411:
URL: https://github.com/apache/spark/pull/34411
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
8. If you want to add or modify an error type or message, please read the guideline first in
'core/src/main/resources/error/README.md'.
-->
### What changes were proposed in this pull request?
Inline type hints for python/pyspark/conf.py
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
Currently, Inline type hints for python/pyspark/conf.pyi doesn't support type checking within function bodies. So we inline type hints to support that.
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
No.
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
### How was this patch tested?
Exising test.
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
-->
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957164787
add to whitelist
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738229515
##########
File path: python/pyspark/conf.py
##########
@@ -122,9 +126,9 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
else:
# JVM is not created, so store data in self._conf first
self._jconf = None
- self._conf = {}
+ self._conf = {} # type: ignore[var-annotated]
Review comment:
We should probably have this annotated at the top level.
```python
_jconf: Optional[JavaObject]
_conf: Optional[Dict[str, str]]
```
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738138944
##########
File path: python/pyspark/conf.py
##########
@@ -178,9 +191,18 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ @overload
Review comment:
I add overload because If I only set the return value as Optional, https://github.com/apache/spark/blob/master/python/pyspark/sql/session.py#L613 will show error.
##########
File path: python/pyspark/conf.py
##########
@@ -195,21 +217,21 @@ def get(self, key, defaultValue=None):
else:
return self._conf.get(key, defaultValue)
- def getAll(self):
+ def getAll(self) -> Union[List[Tuple[str, str]], ItemsView[Any, Any]]:
Review comment:
Is there a more clever way rather than specifying `ItemsView[Any, Any]`?
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
@xinrong-databricks I am not sure how to union the two overload functions.
##########
File path: python/pyspark/conf.py
##########
@@ -122,9 +126,9 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
else:
# JVM is not created, so store data in self._conf first
self._jconf = None
- self._conf = {}
+ self._conf = {} # type: ignore[var-annotated]
Review comment:
Should I ignore it?
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957208321
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49312/
--
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] SparkQA removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957166087
**[Test build #144842 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144842/testReport)** for PR 34411 at commit [`82898e0`](https://github.com/apache/spark/commit/82898e0e01f67d4a7ef537181c38eb6df5c2a625).
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960520864
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49364/
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r741864096
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Unrelated note ‒ shouldn't we use `get` here?
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property."""
# Try to set self._jconf first if JVM is created, set self._conf if JVM is not created yet.
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- self._conf[key] = str(value)
+ cast(Dict[str, str], self._conf)[key] = str(value)
Review comment:
I'd probably go with `assert` here
```patch
diff --git a/python/pyspark/conf.py b/python/pyspark/conf.py
index 09c8e63d09..a8538b06e4 100644
--- a/python/pyspark/conf.py
+++ b/python/pyspark/conf.py
@@ -136,7 +136,8 @@ class SparkConf(object):
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- cast(Dict[str, str], self._conf)[key] = str(value)
+ assert self._conf is not None
+ self._conf[key] = str(value)
return self
def setIfMissing(self, key: str, value: str) -> "SparkConf":
```
but I guess it is fine for now.
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Also, same as above ‒ single `assert` might be a better option.
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Unrelated note ‒ shouldn't we use `get` here?
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property."""
# Try to set self._jconf first if JVM is created, set self._conf if JVM is not created yet.
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- self._conf[key] = str(value)
+ cast(Dict[str, str], self._conf)[key] = str(value)
Review comment:
I'd probably go with `assert` here
```patch
diff --git a/python/pyspark/conf.py b/python/pyspark/conf.py
index 09c8e63d09..a8538b06e4 100644
--- a/python/pyspark/conf.py
+++ b/python/pyspark/conf.py
@@ -136,7 +136,8 @@ class SparkConf(object):
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- cast(Dict[str, str], self._conf)[key] = str(value)
+ assert self._conf is not None
+ self._conf[key] = str(value)
return self
def setIfMissing(self, key: str, value: str) -> "SparkConf":
```
but I guess it is fine for now.
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Also, same as above ‒ single `assert` might be a better option.
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r741866367
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Also, same as above ‒ single `assert` might be a better option.
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960474151
**[Test build #144894 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144894/testReport)** for PR 34411 at commit [`b812d4e`](https://github.com/apache/spark/commit/b812d4e206f8c56b93e8cac8c802773dac53a76e).
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-953486261
Can one of the admins verify this patch?
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738225550
##########
File path: python/pyspark/conf.py
##########
@@ -178,9 +191,18 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ @overload
+ def get(self, key: str, defaultValue: str) -> str:
+ ...
+
+ @overload
+ def get(self, key: str) -> Optional[str]:
Review comment:
To be precise, this can also happen, when `defaultValue` is provided as `None` (`Literal[None]`).
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957186209
**[Test build #144842 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144842/testReport)** for PR 34411 at commit [`82898e0`](https://github.com/apache/spark/commit/82898e0e01f67d4a7ef537181c38eb6df5c2a625).
* 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.
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738234250
##########
File path: python/pyspark/conf.py
##########
@@ -195,21 +217,21 @@ def get(self, key, defaultValue=None):
else:
return self._conf.get(key, defaultValue)
- def getAll(self):
+ def getAll(self) -> Union[List[Tuple[str, str]], ItemsView[Any, Any]]:
Review comment:
Let's avoid `Union` types in return. Even when they're necessary (i.e. `Optional`) they're real pain to use.
At minimum, we can use `Collection[Tuple[str, str]]`, but it might be better to covert in L227
```python
return list(self._conf.items())
```
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739496361
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
@ueshin @HyukjinKwon WDYT?
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957166087
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957211118
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r741864096
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Unrelated note ‒ shouldn't we use `get` here?
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property."""
# Try to set self._jconf first if JVM is created, set self._conf if JVM is not created yet.
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- self._conf[key] = str(value)
+ cast(Dict[str, str], self._conf)[key] = str(value)
Review comment:
I'd probably go with `assert` here
```patch
diff --git a/python/pyspark/conf.py b/python/pyspark/conf.py
index 09c8e63d09..a8538b06e4 100644
--- a/python/pyspark/conf.py
+++ b/python/pyspark/conf.py
@@ -136,7 +136,8 @@ class SparkConf(object):
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- cast(Dict[str, str], self._conf)[key] = str(value)
+ assert self._conf is not None
+ self._conf[key] = str(value)
return self
def setIfMissing(self, key: str, value: str) -> "SparkConf":
```
but I guess it is fine for now.
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r741864096
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Unrelated note ‒ shouldn't we use `get` here?
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property."""
# Try to set self._jconf first if JVM is created, set self._conf if JVM is not created yet.
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- self._conf[key] = str(value)
+ cast(Dict[str, str], self._conf)[key] = str(value)
Review comment:
I'd probably go with `assert` here
```patch
diff --git a/python/pyspark/conf.py b/python/pyspark/conf.py
index 09c8e63d09..a8538b06e4 100644
--- a/python/pyspark/conf.py
+++ b/python/pyspark/conf.py
@@ -136,7 +136,8 @@ class SparkConf(object):
if self._jconf is not None:
self._jconf.set(key, str(value))
else:
- cast(Dict[str, str], self._conf)[key] = str(value)
+ assert self._conf is not None
+ self._conf[key] = str(value)
return self
def setIfMissing(self, key: str, value: str) -> "SparkConf":
```
but I guess it is fine for now.
##########
File path: python/pyspark/conf.py
##########
@@ -178,49 +193,49 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ def get(self, key: str, defaultValue: Optional[str] = None) -> Optional[str]:
"""Get the configured value for some key, or return a default otherwise."""
- if defaultValue is None: # Py4J doesn't call the right get() if we pass None
+ if defaultValue is None: # Py4J doesn't call the right get() if we pass None
if self._jconf is not None:
if not self._jconf.contains(key):
return None
return self._jconf.get(key)
else:
- if key not in self._conf:
+ if key not in cast(Dict[str, str], self._conf):
return None
- return self._conf[key]
+ return cast(Dict[str, str], self._conf)[key]
Review comment:
Also, same as above ‒ single `assert` might be a better option.
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738569123
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
but #L131 requires #L175 `self.set("spark.executorEnv." + key, value)` to have a `value `with type of str, but in this case, the `value` might be None.
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739106537
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
I guess we could be less restrictive here, but I am not sure if that's a good idea, since it would depend on `__str__` or `__repr__` implementation for the `value`. This has some weird consequences, like:
```python
>>> key, value = "foo", None
>>> conf = sc.getConf()
>>> conf.set(key, value)
<pyspark.conf.SparkConf object at 0x7f4870aa5ee0>
>>> conf.get(key) == value
False
```
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738871404
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
why do I need to use string formatting? Is it only for better formatting?
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960494990
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144894/
--
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] SparkQA removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957166087
**[Test build #144842 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144842/testReport)** for PR 34411 at commit [`82898e0`](https://github.com/apache/spark/commit/82898e0e01f67d4a7ef537181c38eb6df5c2a625).
--
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 change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740773747
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
Shall we keep it as `Any` for now?
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957166087
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-955241543
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #34411:
URL: https://github.com/apache/spark/pull/34411
--
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] ByronHsu commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-953606181
@xinrong-databricks @ueshin Can you help me take a look? I refer to `conf.pyi`, but I found that If I copy from pyi entirely, some of them will cause errors. Therefore, I manually modify some types to remove the error, but I am not sure whether it is correct.
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738758715
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
To address that.
- Add `type: ignore[arg-type]` (and possibly `operator`).
- Use`cast(str, _)`
- Add an assertions (`assert _ is nor None`)
For `key` alone, you could also switch to string formatting (`f"spark.executorEnv.{key}"` / `"spark.executorEnv.{}".format(key)`)****
--
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] ByronHsu commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-954403679
@zero323 @xinrong-databricks Thanks for your advice. I have revised my code.
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739416303
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
I will change to string formatting! Thanks!
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-953486261
Can one of the admins verify this patch?
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957166087
**[Test build #144842 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144842/testReport)** for PR 34411 at commit [`82898e0`](https://github.com/apache/spark/commit/82898e0e01f67d4a7ef537181c38eb6df5c2a625).
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740642524
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
@ueshin @HyukjinKwon Could you help me review this patch? Thanks!
--
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] SparkQA removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960474151
**[Test build #144894 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144894/testReport)** for PR 34411 at commit [`b812d4e`](https://github.com/apache/spark/commit/b812d4e206f8c56b93e8cac8c802773dac53a76e).
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960485056
**[Test build #144894 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144894/testReport)** for PR 34411 at commit [`b812d4e`](https://github.com/apache/spark/commit/b812d4e206f8c56b93e8cac8c802773dac53a76e).
* 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.
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739414769
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
How can we solve it? Could you provide more details?
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739417625
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
So do we need to change the type of `value` from `str` to `Optional[str]`? Or we can open another ticket for this issue.
--
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] ByronHsu commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-954405946
Also, I am curious about the advantage of inline-type rather than stub files.
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960508261
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49364/
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-961631788
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] HyukjinKwon closed pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #34411:
URL: https://github.com/apache/spark/pull/34411
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #34411:
URL: https://github.com/apache/spark/pull/34411
--
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 change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740773747
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
Shall we keep it as `Any` for now?
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
tough call .. I would keep it as just `str` for now though .. for `None` it should be mapped to `null` on JVM ideally.
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957164787
add to whitelist
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-961631788
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738226943
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
```python
self, key: Optional[str], value: Optional[str], pairs: Optional[List[Tuple[str, str]])]
```
?
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738235067
##########
File path: python/pyspark/conf.py
##########
@@ -18,10 +18,13 @@
__all__ = ['SparkConf']
import sys
+from typing import overload, ItemsView, Any
+from typing import List, Optional, Tuple, Union
Review comment:
Nit: Let's use single import from `typing` and sort this in lexicographic order.
--
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] zero323 commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-954506300
> Also, I am curious about the advantage of inline-type rather than stub files.
Both have pros and cons.
The biggest advantage here, is that it enables checking actual implementation (other type checkers might check code, even if stubs), so it makes easier to detect discrepancies and you get additional coverage for your code as bonus.
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739003043
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
> why do I need to use string formatting? Is it only for better formatting?
You don't have to, but I'd expect that `key: Optional[str]` will raise `[operator]` on `+`. In contrast, formatting doesn't care, and we already check if `key is not None` in the conditional (however, only assertions `assert foo is not None` are used to support checking).
--
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] dchvn commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
dchvn commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738873431
##########
File path: python/pyspark/conf.py
##########
@@ -105,15 +108,16 @@ class SparkConf(object):
spark.home=/path
"""
- def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
+ def __init__(self, loadDefaults: bool = True, _jvm: Optional[JVMView] = None,
+ _jconf: Optional[JavaObject] = None) -> None:
Review comment:
Some information in https://github.com/python/mypy/pull/5677 and https://mypy.readthedocs.io/en/stable/error_code_list2.html
mypy implicitly None for type checked ```__init__``` when it take at least one argument
--
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] ByronHsu edited a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu edited a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-953606181
@xinrong-databricks @ueshin Can you help me take a look? I refer to `conf.pyi`, but I found that If I copy the type from pyi entirely, some of them will cause errors. Therefore, I manually modify some types to remove the error, but I am not sure whether it is correct.
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738138944
##########
File path: python/pyspark/conf.py
##########
@@ -178,9 +191,18 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ @overload
Review comment:
I add overload because If I set the return value as Optional, https://github.com/apache/spark/blob/master/python/pyspark/sql/session.py#L613 will show error.
##########
File path: python/pyspark/conf.py
##########
@@ -178,9 +191,18 @@ def setAll(self, pairs):
self.set(k, v)
return self
- def get(self, key, defaultValue=None):
+ @overload
Review comment:
I add overload because If I set the return value as Optional, https://github.com/apache/spark/blob/master/python/pyspark/sql/session.py#L613 will show an error.
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740642524
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
@ueshin @HyukjinKwon Could you help me review this patch? Thanks!
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957211118
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144842/
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957251413
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49312/
--
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] zero323 commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-954620482
ok to test
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739003043
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
> why do I need to use string formatting? Is it only for better formatting?
You don't have to, but I'd expect that `key: Optional[str]` will raise `[operator]` on `+`. In contrast, formatting doesn't care, and we already check if `key is not None` in the conditional.
--
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] zero323 commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
zero323 commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738758924
##########
File path: python/pyspark/conf.py
##########
@@ -133,28 +137,37 @@ def set(self, key, value):
self._conf[key] = str(value)
return self
- def setIfMissing(self, key, value):
+ def setIfMissing(self, key: str, value: str) -> "SparkConf":
"""Set a configuration property, if not already set."""
if self.get(key) is None:
self.set(key, value)
return self
- def setMaster(self, value):
+ def setMaster(self, value: str) -> "SparkConf":
"""Set master URL to connect to."""
self.set("spark.master", value)
return self
- def setAppName(self, value):
+ def setAppName(self, value: str) -> "SparkConf":
"""Set application name."""
self.set("spark.app.name", value)
return self
- def setSparkHome(self, value):
+ def setSparkHome(self, value: str) -> "SparkConf":
"""Set path where Spark is installed on worker nodes."""
self.set("spark.home", value)
return self
- def setExecutorEnv(self, key=None, value=None, pairs=None):
+ @overload
+ def setExecutorEnv(self, key: str, value: str) -> "SparkConf":
+ ...
+
+ @overload
+ def setExecutorEnv(self, *, pairs: List[Tuple[str, str]]) -> "SparkConf":
+ ...
+
+ # TODO
+ def setExecutorEnv(self, key=None, value=None, pairs=None): # type: ignore[no-untyped-def]
Review comment:
To address that you can
- Add `type: ignore[arg-type]` (and possibly `operator`).
- Use`cast(str, _)`
- Add an assertions (`assert _ is nor None`)
For `key` alone, you could also switch to string formatting (`f"spark.executorEnv.{key}"` / `"spark.executorEnv.{}".format(key)`)****
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r739417625
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
So do we need to change the type of `value` from `str` to `Optional[str]`? Or could we open another ticket for this issue?
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-955241543
Can one of the admins verify this patch?
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957211118
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144842/
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957264003
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49312/
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957264003
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49312/
--
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 change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740777932
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
tough call .. I would keep it as just `str` for now though .. for `None` it should be mapped to `null` on JVM ideally.
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-955241543
Can one of the admins verify this patch?
--
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-955241543
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960494990
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/144894/
--
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] xinrong-databricks commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
xinrong-databricks commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738837531
##########
File path: python/pyspark/conf.py
##########
@@ -122,9 +126,9 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
else:
# JVM is not created, so store data in self._conf first
self._jconf = None
- self._conf = {}
+ self._conf = {} # type: ignore[var-annotated]
Review comment:
FYI https://www.python.org/dev/peps/pep-0526/
--
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] xinrong-databricks commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
xinrong-databricks commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-954301150
Thanks, @ByronHsu! It is expected that type hints in `pyi` files cannot be inlined directly. Especially, after inlining type hints, function bodies will be type-checked as well. Let's either adjust type hints or ignore the respective mypy errors.
--
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] xinrong-databricks commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
xinrong-databricks commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738844261
##########
File path: python/pyspark/conf.py
##########
@@ -105,15 +108,16 @@ class SparkConf(object):
spark.home=/path
"""
- def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
+ def __init__(self, loadDefaults: bool = True, _jvm: Optional[JVMView] = None,
+ _jconf: Optional[JavaObject] = None) -> None:
Review comment:
It seems we don't need to annotate the return type of `__init__`.
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740642524
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
@ueshin @HyukjinKwon Could you help me review this patch? Thanks!
--
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] ByronHsu commented on a change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r738868622
##########
File path: python/pyspark/conf.py
##########
@@ -105,15 +108,16 @@ class SparkConf(object):
spark.home=/path
"""
- def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
+ def __init__(self, loadDefaults: bool = True, _jvm: Optional[JVMView] = None,
+ _jconf: Optional[JavaObject] = None) -> None:
Review comment:
why is that?
--
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] AmplabJenkins commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957211118
--
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 change in pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34411:
URL: https://github.com/apache/spark/pull/34411#discussion_r740773747
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
Shall we keep it as `Any` for now?
##########
File path: python/pyspark/conf.py
##########
@@ -124,48 +130,57 @@ def __init__(self, loadDefaults=True, _jvm=None, _jconf=None):
self._jconf = None
self._conf = {}
- def set(self, key, value):
+ def set(self, key: str, value: str) -> "SparkConf":
Review comment:
tough call .. I would keep it as just `str` for now though .. for `None` it should be mapped to `null` on JVM ideally.
--
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] SparkQA removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957166087
**[Test build #144842 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/144842/testReport)** for PR 34411 at commit [`82898e0`](https://github.com/apache/spark/commit/82898e0e01f67d4a7ef537181c38eb6df5c2a625).
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-957164787
add to whitelist
--
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 #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-961631788
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] AmplabJenkins removed a comment on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960520864
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/49364/
--
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] ByronHsu commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
ByronHsu commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960462701
@zero323 I have updated the code. Thank you for your precious advice!
--
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] SparkQA commented on pull request #34411: [SPARK-37137][PYTHON] Inline type hints for python/pyspark/conf.py
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #34411:
URL: https://github.com/apache/spark/pull/34411#issuecomment-960491667
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49364/
--
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