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 compatibility #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thijstriemstra
Copy link
Contributor

I tried porting my own library to Python 3(.4), that depends on tower, and fixed some Python 3 errors in tower along the way. Unfortunately it's not yet possible to run the test suite under Python 3 because the translate-toolkit library is not compatible with Python 3 yet it seems, and tower depends on it.

Also updated some of the dependencies in requirements.txt and added six (comes with Django 1.6 and later).

run_tests.py now also works with Django 1.7.

babel==1.3
jinja2
six
Copy link
Owner

Choose a reason for hiding this comment

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

Can we specify a version? Probably will never be a problem, but just in case.

@clouserw
Copy link
Owner

Thanks - this looks great. And thanks for adding the tests. Can you rebase this into one commit?

@thijstriemstra
Copy link
Contributor Author

Squashed the commits into one, never did that before, git's cool.

@clouserw
Copy link
Owner

I wonder if our testing is set up correctly. On the Travis page, if you click through to the python2.7 tests it says it's using 2.6: https://travis-ci.org/clouserw/tower/builds/55825211

@magopian - what do you think?

@thijstriemstra
Copy link
Contributor Author

I ran into that issue before and solved it with $TRAVIS_PYTHON_VERSION, for example:

language: python
python:
  - "2.7"
  - "3.4"
install:
  - "pip install -r requirements/production.txt"
  - "pip install -r requirements/testing.txt"
  - "pip install ."
script:
  - if [[ $TRAVIS_PYTHON_VERSION == '2.7' ]]; then tox -e py27,coverage; fi
  - if [[ $TRAVIS_PYTHON_VERSION == '3.4' ]]; then tox -e py34; fi

@magopian
Copy link
Contributor

The interpreter version that's displayed in travis is the one used by the travis.yml file. But this file is only used to "proxy" to tox: https://github.com/clouserw/tower/pull/32/files#diff-354f30a63fb0907d4ad57269548329e3L13

What you want, though, is at least a target (an environment) for python3, if you're porting to py3, just to make sure that it does run correctly on py3.

I therefore believe that it's missing:

[testenv:py27-1.7]
basepython = python2.7
deps =
    Django==1.7.7
    {[testenv]deps}
{[testenv]deps}

@thijstriemstra
Copy link
Contributor Author

The python 3 builder will fail until the tower dependencies like translate-toolkit are ported. Making sure that tower is ready for it is what this PR is about.

@magopian
Copy link
Contributor

Ah, that's a hard problem to solve then, chicken and egg of some sorts.
How did you make sure the code was compatible (and ran correctly) under py3 if the dependencies aren't ported yet?

If you did it by also porting the other dependencies, it means you have a fork or something? If so, maybe you can (temporarily, for the sake or running the tests) change the requirements to point to your py3 ported fork?

I personally don't usually port the code from a lib before its dependencies are already ported, to avoid this kind of issue ;)

@thijstriemstra
Copy link
Contributor Author

I ran 2to3 on the codebase, fixed the django 1.7 build/test runner and tried to fix the failing tests for python 3.4 but realized it the dependencies that caused the failures. So now we have something that has improved but can't be measured, so should we throw this PR away? :)

There is some flag in travis that allows you to have failing builds. Maybe setting up a 3.4 builder with that enabled helps us see when dependencies start supporting python 3?

@clouserw
Copy link
Owner

Thanks for all the info. Yeah, we should probably not land this until we can make sure it works. I've been pretty removed from the toolkit. It looks like Dwayne had a py3 branch a year ago at https://github.com/translate/translate/tree/feature/py3 . Maybe you could try to sync up with him if you're still in the py3 mood :)

@thijstriemstra
Copy link
Contributor Author

I see.

@jogo jogo mentioned this pull request Dec 16, 2017
@thijstriemstra
Copy link
Contributor Author

I've always been baffled how this project not only has one, but two PRs, trying to add Python 3 support and it's just crickets... Sad!

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.

3 participants