Skip to content
This repository was archived by the owner on Dec 29, 2022. It is now read-only.

Changes for compatibility with Python 3 #22

Closed
wants to merge 2 commits into from
Closed

Changes for compatibility with Python 3 #22

wants to merge 2 commits into from

Conversation

joachimmetz
Copy link
Member

Changes for compatibility with Python 3

@PhillipSz
Copy link

Hey, can some please review this patch? Thank you!

@@ -325,7 +325,7 @@ def __init__(self, list_sep):

def Serialize(self, value):
"""Serialize a list as a string, if possible, or as a unicode string."""
output = cStringIO.StringIO()
output = io.StringIO()
writer = csv.writer(output)

# csv.writer doesn't accept unicode, so we convert to UTF-8.
Copy link

@barabo barabo Jun 23, 2016

Choose a reason for hiding this comment

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

I believe there is a lurking bug on the following line when running in python3 (use of unicode keyword).

Instead, I believe you can replace all occurrences (there's only one) of CsvListSerializer(',') with ListSerializer(','). This prevents the need for importing the io module entirely.

vrusinov added a commit that referenced this pull request Jul 17, 2016
It is declares python2-only at the momens since there are still issues with
python3 support.

Related:
 - github pull request #22
 - github pull request #26
 - github pull request #29
 - github issue #28
 - github issue #20

	Change on 2016/06/20 by vrusinov <[email protected]>

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125322335
@gpshead
Copy link
Contributor

gpshead commented Nov 15, 2016

take a look at the recent pushes, it should be working on Python 3.

@joachimmetz
Copy link
Member Author

take a look at the recent pushes, it should be working on Python 3.

Thanks I'll close this PR then.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants