django-media-tree icon indicating copy to clipboard operation
django-media-tree copied to clipboard

python3 compatibility fixes

Open gbezyuk opened this issue 10 years ago • 9 comments
trafficstars

gbezyuk avatar Oct 11 '15 23:10 gbezyuk

There are some weird changes like extra spaces or "changing text with exactly the same one", I tried to get rid of them but did not succeed. Anyway, it shouldn't matter.

gbezyuk avatar Oct 11 '15 23:10 gbezyuk

Just a comment: How can we make sure these changes actually make django-media-tree work with Python 3? And then again, which versions of Python 3?

@samluescher I had added tox configuration and badges in PR #40, but in fact there are no tests to run. So, we can't actually test the code, be it existing code or changes, against any target.

Can we come up with a very global function test covering a lot of media tree's feature set, with limited effort? (i.e. no unit tests, they can come later)

bittner avatar Oct 12 '15 09:10 bittner

@bittner there are no tests to check the project with, that's true. Actually, I came with these changes while trying to run django-media-tree against python3 and django1.8.4. (Didn't succeed with this version of Django yet, BTW — thus there are no django-related changes in this PR.)

There are only three types of changes in this PR:

  • exception handling syntax (ExceptionType 'as' exception, not ExceptionType, exception)
  • unicode-related imports (smart_text instead of smart_unicode, keeping python2 names in the rest of a code)
  • minor syntax fixes like print arg to pring(arg) and unichr() to from builtins import chr <...> chr()

All of these at least shouldn't break anything, and clearly moves the codebase closer to python3 readiness — due to absence of tests, unfortunately I can't help with much more.

gbezyuk avatar Oct 12 '15 10:10 gbezyuk

@gbezyuk I don't doubt it. Really, I love seeing projects move closer to Python 3. Though, how can we make sure media tree remains compatible with all supported Python 2 (and Django) versions? Without any tests? We can't.

Have you tried running the demo project included in the repository? (I must admit, I haven't.) -- I'd suggest to build the demo project for each and every Python-Django combination and run basic function tests [1] against them. That shouldn't be too much of a pain. The tox configuration is already in place. And Travis-CI is waiting for another open source project to burn their CPU power! :smirk:

[1] Running python manage.py migrate and python manage.py check would already be "tests"!

bittner avatar Oct 12 '15 10:10 bittner

Well, actually with python3 the demo project fails on pip install -r requirements step with errors like print "Setuptools version",version,"or greater has been installed." SyntaxError: Missing parentheses in call to 'print' while installing wsgiref and easy-thumbnails. Can't do much with this without hacking deep into the project, too.

With python2.7, however, it still works as expected, so I doubt if I broke anything.

gbezyuk avatar Oct 12 '15 10:10 gbezyuk

Don't worry, I'm not blaming you on braking anything. We should make the demo project deploy (as a test) on Travis-CI then we're all set. I'll try. -- Thanks for the discussion!

bittner avatar Oct 12 '15 10:10 bittner

Thank you too) hope I triggered some good changes ;)

gbezyuk avatar Oct 12 '15 11:10 gbezyuk

I would also add this for python 3.5 compatibility

try:
    from builtins import chr as unichr
except ImportError:
    pass

SebCorbin avatar Nov 04 '15 11:11 SebCorbin

@gbezyuk The print complaints are easily fixed in the demo project. We just need to use print() as a function instead of a keyword. Works both in 2.7 and 3.x. Ooops, not if it comes from incompatible modules when they're installed.

Looks like the setup of the demo project needs some extra work. And so probably will the making-compatible-with-newer-versions-of-Django/Python-3.x. The first step needed is merging PR #42. @samluescher Any chance?

bittner avatar Jan 06 '16 21:01 bittner