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

Python 3.3 compatibility #44

Merged
merged 3 commits into from
May 15, 2013
Merged

Python 3.3 compatibility #44

merged 3 commits into from
May 15, 2013

Conversation

matklad
Copy link
Contributor

@matklad matklad commented May 13, 2013

I've added python 3.3 compatibility.

  • explicit relative imports
  • force_text instead of force_unicode(except for django 1.3)
  • response.unicode_normal_body instead of response.content

And python3.3 tox environment:

[testenv:py33]
basepython=python3.3
deps=
django==1.5.1
South==0.7.6
coverage==3.6

Hope, that it'll be useful =)

@treyhunner
Copy link
Member

This is great. Thanks @matklad!

The __unicode__ method created for historical record models should probably be made into a __str__ method (python_2_unicode_compatible should probably be used in that case).

Do you think we should use from __future__ import unicode_literals in every file with string literals? It seems best to ensure that Python 2 and Python 3 interpret all strings the same way (as unicode or not) to avoid bugs in particular Python versions.

`type` in python2 needs a str though, so an explicit cast is models.py
@matklad
Copy link
Contributor Author

matklad commented May 13, 2013

The unicode method created for historical record models should probably be made into a str method (python_2_unicode_compatible should probably be used in that case).

Yes, this definitely needs a fix, but I'm not sure, which way is right. python_2_unicode_compatible is only available in django 1.5, and django.utils.six only in django >= 1.4.2. Possible fixes are:

  • introduce six as a dependency.
  • abandon django 1.3 support and use six from django.
  • use sys.version_info[0] manualy.

Do you think we should use from future import unicode_literals in every file with string literals?

It's a good idea. I added it to every non empty file(b9609e9). Everything went smoothly except the invocation of type in models.py, where I had to cast first argument to str manually.

@treyhunner
Copy link
Member

We should also be able to support Python 3.2 now with all those u prefixes gone.

I don't think we should add six as a dependency. We could drop Django 1.3 support, but it would probably be best to keep it around for a while for those migrating a code base that used to use one of the older forks of django-simple-history. We could write our own python_2_unicode_compatible-like utility using sys.version_info. The implementation is pretty simple.

@treyhunner
Copy link
Member

This looks good to me and it passes the tests in all listed versions of Python. Is this ready merge?

@matklad
Copy link
Contributor Author

matklad commented May 15, 2013

YeS!

treyhunner added a commit that referenced this pull request May 15, 2013
Python 3.3 compatibility
@treyhunner treyhunner merged commit 027697f into jazzband:master May 15, 2013
@matklad matklad deleted the py33 branch May 15, 2013 16:09
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.

None yet

2 participants