Skip to content
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.

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

resolve python2 str() issue #4030

Merged
merged 5 commits into from
Dec 9, 2017

Conversation

timifasubaa
Copy link
Contributor

@timifasubaa timifasubaa commented Dec 7, 2017

Some slices with utf8 characters are breaking during the string conversion in the lines changed. This removes the str and uses six.text_type so it stays unicode in python2 and becomes str in python3.

@john-bodley @mistercrunch

  File "/usr/local/lib/python2.7/dist-packages/superset/viz.py", line 277, in get_payload
    data = self.get_data(df)
  File "/usr/local/lib/python2.7/dist-packages/superset/viz.py", line 1254, in get_data
    x = str(x)
UnicodeEncodeError: 'ascii' codec can't encode character u'\u2019' in position 11: ordinal not in range(128)```

superset/viz.py Outdated
@@ -1300,9 +1300,9 @@ def get_data(self, df):
for i, v in ys.iteritems():
x = i
if isinstance(x, (tuple, list)):
x = ', '.join([str(s) for s in x])
x = ', '.join([str(s.encode('utf8')) for s in x])
Copy link
Member

Choose a reason for hiding this comment

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

I think you want to use six.text_type(s) instead.

Copy link
Member

Choose a reason for hiding this comment

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

time to upgrade to py3!

Copy link
Contributor Author

@timifasubaa timifasubaa Dec 8, 2017

Choose a reason for hiding this comment

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

@john-bodley I tried six.text_type but it didn't work. python 2 str uses ascii encoding scheme by default so it will break unless utf-8 is specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@john-bodley I removed the str(). six.text_type makes sense now.

Copy link
Contributor

@xrmx xrmx left a comment

Choose a reason for hiding this comment

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

You may also need to add the utf-8 encoding header at the top
# -*- coding: utf-8 -*-

@timifasubaa
Copy link
Contributor Author

@xrmx I think that's for when there are non-ascii characters in the python file and that's not the case here.

@john-bodley
Copy link
Member

LGTM

@timifasubaa timifasubaa changed the title make string conversion use utf8 resolve python2 str() issue Dec 9, 2017
@mistercrunch mistercrunch merged commit 3ed8f5f into apache:master Dec 9, 2017
michellethomas pushed a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* make string conversion use utf8

* Update viz.py

* make utf-8 consistent with other usages

* Update viz.py

* Update viz.py
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* make string conversion use utf8

* Update viz.py

* make utf-8 consistent with other usages

* Update viz.py

* Update viz.py
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.2 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants