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

Support for Python 2.7 #2

Merged
merged 9 commits into from
Jan 10, 2018
Merged

Support for Python 2.7 #2

merged 9 commits into from
Jan 10, 2018

Conversation

kayabe
Copy link
Contributor

@kayabe kayabe commented Jan 7, 2018

No description provided.

@kayabe
Copy link
Contributor Author

kayabe commented Jan 7, 2018

could you check out if something is wrong with these commits on python 2.7?

@igorcmoura
Copy link
Owner

Really thank you for your contribution!
In general it works, but there are some little problems.

@@ -3,7 +3,7 @@
from unittest import TestCase

import anitopy
from tests.fixtures.table import table
Copy link
Owner

Choose a reason for hiding this comment

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

The tests fail for python 3 because of the removal of tests. on the imports. You could keep it and it would still works on python 2.7 if you used the discover argument to run unittest.
With this little fix, python 3 passes all tests, but python 2 fails in some, all because of unicode problems. To fix this you just have to import unicode_literals from __future__ in all files (except __init__.py) to convert all strings to unicode. As demonstrated here.

string = unicode(string, 'utf-8')
except:
pass

Copy link
Owner

Choose a reason for hiding this comment

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

Importing unicode_literals you can remove this unicode conversion.

@@ -53,6 +53,12 @@ def is_dash_character(string):


def is_latin_char(char):
if type(char) == str:
try:
char = unicode(char, 'utf-8')
Copy link
Owner

Choose a reason for hiding this comment

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

And also this

@igorcmoura
Copy link
Owner

And I think that's all to be fixed!

There are also some improvements you can do. If you don't want to, no problem, I can do it later:

@kayabe
Copy link
Contributor Author

kayabe commented Jan 10, 2018

could you check again?

@igorcmoura
Copy link
Owner

Everything seems perfect!

I gonna merge and release a new version as soon as possible.

@kayabe
Copy link
Contributor Author

kayabe commented Jan 10, 2018

Thanks

@igorcmoura igorcmoura merged commit 38524e8 into igorcmoura:master Jan 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants