Skip to content

[SPARK-33415][PYTHON][SQL] Don't encode JVM response in Column.__repr__ #30322

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

[SPARK-33415][PYTHON][SQL] Don't encode JVM response in Column.__repr__ #30322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49543e8
Don't encode JVM response in Column.__repr__
zero323 Nov 10, 2020
88f0c50
Add test
zero323 Nov 10, 2020
ec72805
Drop blank line
zero323 Nov 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion python/pyspark/sql/column.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ def __nonzero__(self):
__bool__ = __nonzero__

def __repr__(self):
return 'Column<%s>' % self._jc.toString().encode('utf8')
return "Column<'%s'>" % self._jc.toString()
Copy link
Member

@maropu maropu Nov 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python3 uses utf8 for strings by default, so this change seems fine. cc: @HyukjinKwon @viirya @srowen

Copy link
Member

@srowen srowen Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any more instances of decode()?

Copy link
Member

@HyukjinKwon HyukjinKwon Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the change looks good, and yeah, are there more instances like this?

Copy link
Member

@viirya viirya Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. Is it originally for non printable characters in unicode?

Copy link
Member Author

@zero323 zero323 Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any more instances of decode()?

We do a bit of encoding / decoding when we communicate with JVM, but purpose there is clear.

The only other place when we encode strings intended for user consumption is RDD.toDebugString. It also something that could be fixed, as it messing with the output a bit (as print won't respect line breaks).

Seems fine. Is it originally for non printable characters in unicode?

I believe the point was to have str object as the output, instead of unicode. If I recall correctly, unicode (py4j returns JVM Strings as unicode in Python 2 and as a result the whole expression would evaluate to unicode) in __repr__, wasn't handled correctly.

Copy link
Member Author

@zero323 zero323 Nov 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example in IPython

>>> import sys
>>> sys.version_info
sys.version_info(major=2, minor=7, micro=15, releaselevel='final', serial=0)
>>> class Foo:
...     def __repr__(self):
...         return u"œ"
...     
...     
>>> Foo().__repr__()
u'\u0153'
>>> Foo()
Traceback (most recent call last):
  File "/path/to/lib/python2.7/site-packages/IPython/core/formatters.py", line 686, in __call__
    return repr(obj)
UnicodeEncodeError: 'ascii' codec can't encode character u'\u0153' in position 0: ordinal not in range(128)



def _test():
Expand Down
1 change: 1 addition & 0 deletions python/pyspark/sql/tests/test_column.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ def test_column_name_with_non_ascii(self):
self.assertEqual([("数量", 'bigint')], df.dtypes)
self.assertEqual(1, df.select("数量").first()[0])
self.assertEqual(1, df.select(df["数量"]).first()[0])
self.assertTrue(columnName in repr(df[columnName]))

def test_field_accessor(self):
df = self.sc.parallelize([Row(l=[1], r=Row(a=1, b="b"), d={"k": "v"})]).toDF()
Expand Down