JoshRosen commented on a change in pull request #25142: [SPARK-28378][Python]
Remove usage of cgi.escape
URL: https://github.com/apache/spark/pull/25142#discussion_r303214072
##########
File path: python/pyspark/sql/dataframe.py
##########
@@ -375,7 +375,12 @@ def _repr_html_(self):
by 'spark.sql.repl.eagerEval.enabled', this only called by REPL you are
using support eager evaluation with HTML.
"""
- import cgi
+ try:
Review comment:
I think we have some version-specific imports up at the top of this file,
e.g.
https://github.com/apache/spark/blob/560aa43736dcf85f7e49391ce8fe0a8da8d30f69/python/pyspark/sql/dataframe.py#L21
so maybe we could group those there?
On the other hand, the current behavior is to only load `cgi` if
`_repr_html_` is invoked and this PR's changes preserve that behavior, whereas
moving it to the top would cause it to be unconditionally imported.
Unconditional import is fine if this import is generally side-effect-free.
Don't feel strongly, just wanted to note that.
Regardless of whether we move, should we do an explicit `sys.version` check
here instead of the `try except`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
