You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/07/28 12:47:17 UTC
[GitHub] [spark] physinet opened a new pull request, #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
physinet opened a new pull request, #37329:
URL: https://github.com/apache/spark/pull/37329
### What changes were proposed in this pull request?
Support either literal Python strings or Column objects for the pattern and replacement arguments for `regexp_replace`.
### Why are the changes needed?
Allows using different replacements per row, as in [this example](https://stackoverflow.com/questions/64613761/in-pyspark-using-regexp-replace-how-to-replace-a-group-with-value-from-another,).
### Does this PR introduce _any_ user-facing change?
Users can now use `regexp_replace` with columns for all three arguments. The first argument (string that regex should be applied to) can be either a Column object or the string name of the column. In summary, the following signatures are supported:
```python
regexp_replace("str", "\d", "")
regexp_replace(F.col("str"), "\d", "")
regexp_replace("str", F.col("pattern"), F.col("replacement"))
regexp_replace(F.col("str"), F.col("pattern"), F.col("replacement"))
```
### How was this patch tested?
Added unit tests
--
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] physinet commented on a diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r933229230
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Added Parameters to the docstring: [45cd79d](https://github.com/apache/spark/pull/37329/commits/45cd79ddd205ff317f0061fdcf19972f84bbee91)
The types make it somewhat unclear that pattern and replacement must be either _both_ strings or _both_ columns to match the signatures [here](https://github.com/apache/spark/blob/775b5ced70c87d6a8709f44eb4a5de48c286a51d/sql/core/src/main/scala/org/apache/spark/sql/functions.scala#L2826). This is why I had the overloaded signatures. I removed them and tried to clarify in the docstring instead.
--
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 #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1199547429
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] physinet commented on a diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r934545923
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Great ideas. I implemented these in [https://github.com/apache/spark/pull/37329/commits/0bdd92bd07b4d5661049f594bf34d0595ab019c4](0bdd92b). Black formatting pushed the if-else statements onto three lines, which I thought was less readable. I changed the formatting a bit to help with readability.
--
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 #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1201990422
@physinet mind enabling https://github.com/physinet/spark/actions/workflows/build_main.yml and rebasing please? Apache Spark leverages the Github resources from the PR author's fork.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r932809698
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Can we write up `Parameters` section in the docs? And I think you can only keep `pattern: Union[str, Column]` and remove the ones above.
--
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] physinet commented on pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1199252901
Any advice to solve the failing check?
```
Error: Unhandled error: Error: There was a new unsynced commit pushed. Please retrigger the workflow.
```
--
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] physinet commented on a diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r934545923
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Great ideas. I implemented these in [https://github.com/apache/spark/commit/0bdd92bd07b4d5661049f594bf34d0595ab019c4](0bdd92b). Black formatting pushed the if-else statements onto three lines, which I thought was less readable. I changed the formatting a bit to help with readability.
--
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 #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
zero323 commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1221335064
Merged into 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 commented on pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1205895810
I am fine with this. I will leave it to @zero323
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 closed pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
zero323 closed pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
URL: https://github.com/apache/spark/pull/37329
--
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 #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1198809042
cc @zero323 FYI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] physinet commented on a diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r934545923
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Great ideas. I implemented these in 0bdd92b. Black formatting pushed the if-else statements onto three lines, which I thought was less readable. I changed the formatting a bit to help with readability.
--
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 diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
zero323 commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r933778937
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
> The types make it somewhat unclear that pattern and replacement must be either both strings or both columns to match the signatures
Yeah, I would go with `overload` here as well. We capture these types of dependencies in other places and I don't see any reason to make type checks weaker here.
--
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 diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
zero323 commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r933780936
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,7 +3262,19 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Technically speaking we could also default to invoking `(Column, Column) -> Column` and convert arguments accordingly, i.e.
```python
return _invoke_function(
"regexp_replace",
_to_java_column(string),
_create_column_from_literal(pattern) if isinstance(pattern, str) else _to_java_column(pattern),
_create_column_from_literal(replacement) if isinstance(replacement, str) else _to_java_column(replacement),
)
```
--
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] physinet commented on pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1205255228
@HyukjinKwon @zero323 are you happy with the latest changes?
--
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] physinet commented on pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
physinet commented on PR #37329:
URL: https://github.com/apache/spark/pull/37329#issuecomment-1221136223
All comments are addressed; this should be ready for merge.
--
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 diff in pull request #37329: [SPARK-39832][PYTHON] Support column arguments in regexp_replace
Posted by GitBox <gi...@apache.org>.
zero323 commented on code in PR #37329:
URL: https://github.com/apache/spark/pull/37329#discussion_r949968025
##########
python/pyspark/sql/functions.py:
##########
@@ -3262,18 +3262,49 @@ def regexp_extract(str: "ColumnOrName", pattern: str, idx: int) -> Column:
return _invoke_function("regexp_extract", _to_java_column(str), pattern, idx)
-def regexp_replace(str: "ColumnOrName", pattern: str, replacement: str) -> Column:
- r"""Replace all substrings of the specified string value that match regexp with rep.
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: str, replacement: str) -> Column:
+ ...
+
+
+@overload
+def regexp_replace(string: "ColumnOrName", pattern: Column, replacement: Column) -> Column:
+ ...
+
+
+def regexp_replace(
+ string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
Review Comment:
Nit. With the latest changes we can drop overloaded signatures and keep only
```python
def regexp_replace(
string: "ColumnOrName", pattern: Union[str, Column], replacement: Union[str, Column]
) -> Column:
```
Otherwise LGTM
--
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