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.

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

Fix encoding bugs in Python2 (non-ASCII characters) #80

Merged
merged 7 commits into from
Sep 19, 2016
Merged

Fix encoding bugs in Python2 (non-ASCII characters) #80

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6a0bba
Merge pull request #39 from RussTheAerialist/master
Jun 29, 2016
59ac21c
Merge pull request #66 from tableau/development
Jul 26, 2016
bcd73c1
Update README.md (#71)
chid Aug 15, 2016
9a098a7
Merge pull request #78 from tableau/development
Aug 31, 2016
3fc9628
Fix bug in Python2 when column name has non-ascii characters.
MiguelSR Sep 8, 2016
473d613
Fix get description method for Python 2 when description has non-ASCI…
MiguelSR Sep 8, 2016
8de7409
Small refactor, check type of string instead of python version
MiguelSR Sep 19, 2016
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 tableaudocumentapi/datasource.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
def _get_metadata_xml_for_field(root_xml, field_name):
if "'" in field_name:
field_name = sax.escape(field_name, {"'": "'"})
xpath = ".//metadata-record[@class='column'][local-name='{}']".format(field_name)
xpath = u".//metadata-record[@class='column'][local-name='{}']".format(field_name)
return root_xml.find(xpath)


Expand Down
8 changes: 7 additions & 1 deletion tableaudocumentapi/field.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import functools
import sys
import xml.etree.ElementTree as ET


Expand Down Expand Up @@ -199,4 +200,9 @@ def _read_description(xmldata):
if description is None:
return None

return u'{}'.format(ET.tostring(description, encoding='utf-8')) # This is necessary for py3 support
description_string = ET.tostring(description, encoding='utf-8')
# Format expects a unicode string so in Python 2 we have to do the explicit conversion
if sys.version_info[0] == 2:
description_string = description_string.decode('utf-8')

return u'{}'.format(description_string) # This is necessary for py3 support
Copy link
Contributor

@t8y8 t8y8 Sep 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Can this be done with a typecheck instead?

if isinstance(description_string, bytes):
    description_string = description_string.decode('utf-8')
return description_string

Then I believe the u'{}' format dance isn't necessary

(And we don't have to import sys and do the version check)

If we must do a version check, can you put it at the top

PY2 = sys.version_info[0] == 2
...
if PY2:
    <format dance>

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hello.

Yes it makes sense to check the type instead of the Python version.

I'll sign later the CLA, do the rebase and fix the commit to get rid of sys.

Thanks,
Miguel.