You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "itholic (via GitHub)" <gi...@apache.org> on 2024/04/25 05:51:12 UTC
[PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
itholic opened a new pull request, #46215:
URL: https://github.com/apache/spark/pull/46215
### What changes were proposed in this pull request?
This PR followups for https://github.com/apache/spark/pull/46063 to exclude Python magic methods from error context target
### Why are the changes needed?
We only need to logging the Apache Spark Column API functions for DataFrameQueryContext, but not Python native functions.
### Does this PR introduce _any_ user-facing change?
No API changes, but the error message only contain Column APIs that are supported by Apache Spark
### How was this patch tested?
The existing CI should pass.
### Was this patch authored or co-authored using generative AI tooling?
<!--
If generative AI tooling has been used in the process of authoring this patch, please include the
phrase: 'Generated-by: ' followed by the name of the tool and its version.
If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
No.
--
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
Re: [PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #46215:
URL: https://github.com/apache/spark/pull/46215#discussion_r1578936273
##########
python/pyspark/errors/utils.py:
##########
@@ -197,6 +197,16 @@ def with_origin_to_class(cls: Type[T]) -> Type[T]:
"""
if os.environ.get("PYSPARK_PIN_THREAD", "true").lower() == "true":
for name, method in cls.__dict__.items():
- if callable(method) and name != "__init__":
+ # Excluding Python magic methods that do not utilize JVM functions.
+ if callable(method) and name not in (
+ "__init__",
+ "__new__",
+ "__getattr__",
+ "__getitem__",
Review Comment:
Ah, yes we should remove `__getitem__` from the list but can we keep `__getattr__` here as it is?
Because `__getattr__` eventually calls `__getitem__` so the call site always overwritten by `__getitem__` in the end.
--
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
Re: [PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #46215:
URL: https://github.com/apache/spark/pull/46215#discussion_r1579304364
##########
python/pyspark/errors/utils.py:
##########
@@ -197,6 +197,16 @@ def with_origin_to_class(cls: Type[T]) -> Type[T]:
"""
if os.environ.get("PYSPARK_PIN_THREAD", "true").lower() == "true":
for name, method in cls.__dict__.items():
- if callable(method) and name != "__init__":
+ # Excluding Python magic methods that do not utilize JVM functions.
+ if callable(method) and name not in (
+ "__init__",
+ "__new__",
+ "__getattr__",
+ "__getitem__",
Review Comment:
If that's the case, I think we should list up all the aliases 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
Re: [PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #46215:
URL: https://github.com/apache/spark/pull/46215#issuecomment-2078831120
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
Re: [PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #46215: [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target
URL: https://github.com/apache/spark/pull/46215
--
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
Re: [PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #46215:
URL: https://github.com/apache/spark/pull/46215#discussion_r1580282285
##########
python/pyspark/errors/utils.py:
##########
@@ -197,6 +197,16 @@ def with_origin_to_class(cls: Type[T]) -> Type[T]:
"""
if os.environ.get("PYSPARK_PIN_THREAD", "true").lower() == "true":
for name, method in cls.__dict__.items():
- if callable(method) and name != "__init__":
+ # Excluding Python magic methods that do not utilize JVM functions.
+ if callable(method) and name not in (
+ "__init__",
+ "__new__",
+ "__getattr__",
+ "__getitem__",
Review Comment:
Seems like all the Column APIs directly calls the Spark API by itself other than `__getattr__`. But I think we can just remove it as well to keep consistency.
--
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
Re: [PR] [SPARK-47858][PYTHON][FOLLOWUP] Excluding Python magic methods from error context target [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #46215:
URL: https://github.com/apache/spark/pull/46215#discussion_r1578922375
##########
python/pyspark/errors/utils.py:
##########
@@ -197,6 +197,16 @@ def with_origin_to_class(cls: Type[T]) -> Type[T]:
"""
if os.environ.get("PYSPARK_PIN_THREAD", "true").lower() == "true":
for name, method in cls.__dict__.items():
- if callable(method) and name != "__init__":
+ # Excluding Python magic methods that do not utilize JVM functions.
+ if callable(method) and name not in (
+ "__init__",
+ "__new__",
+ "__getattr__",
+ "__getitem__",
Review Comment:
I think `__getattr__` and `__getitem__` is still a PySpark expression? like `functions.getItem`
--
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