Changes made to be able to install worldengine using Python 3.4.

[tcld]

I finally got the code to run (thanks a lot to your help!). I had to make a couple changes to make it work with Python 3, though. With Python 2.7 I wasn't even able to install the necessary dependencies.

Some of the changes are a bit "intrusive", I think - here a short rundown: -every "xrange()" replaced with "range()" (xrange is not available in Python 3) - at worst a slowdown for Python 2 -some import paths changed (e.g.: plates.py -> from "world import World" to "from worldengine.world import World") - not sure about consequences for Python 2 -some print()s needed brackets - as far as I know Python 2 has supported this for some time; problems should not occur -a package imex/export.py is not part of the master-branch, I had to comment out two lines of code - resulting problems for Python 2 and 3 should be the same (and need a fix, I guess) -protobuf-dependency for Python 3 changed to version 3.0.0a3 - the change in setup.py could be problematic for Python 2; maybe the version requirements can be >= instead of ==?

[ftomassetti]

Travis and AppVeyor seems happy: I am not sure if this is due to the fact that we have poor code coverage or it does mean that everything is ok :)

We stopped supporting Python 3 because of several issues including one with protobuf (some version was working only with Python 2).

For range/xrange I think we used to have a definition (one was defined using the other one for either Python 2 or 3 I do not remember).

I would say we should activate CI also for Python 3. To do so we should touch:

• https://github.com/Mindwerks/worldengine/blob/master/.travis.yml
• https://github.com/Mindwerks/worldengine/blob/master/appveyor.yml

To verify all tests run on clean machines under Linux and Windows.

[ftomassetti]

...and thank you so much for sending this one! This is greatly appreciated

[tcld]

I never had to work with unittests (so I don't know what to do to improve the coverage) nor do I have a clue of what to do with the yml files. Any bit of help would be appreciated. :)

And yes, protobuf 2.6.0 was only working with Python 2 (and thus the required dependency on 2.6.0 did not allow using Python 3 at all). I replaced it with the newest version available, even though as far as I know a version >2.6.0 should suffice. To quote an edit I made above: Maybe the version requirements can be >= instead of ==?

An xrange/range definition sounds good. I thought about it yesterday but didn't pursue that path. How should that be approached in a clean manner?

[psi29a]

Great job here! :) I've pushed off python3 until protobuf was ready for python3... and it looks like it is.

YML is Yet another Markup Language, no worries there. You can play with in another branch if you want and send a PR that reads "JUST TESTING, DO NOT MERGE". This is the best way to test your changes in to the yml file.

Unittests are very important, to get a feel for them... just read the code and see that it is mostly calling functions with expected return values. You can either run:

nosetest tests

or

pip install pytest
py.test tests

I'll check out your branch and test it out, if things work good here I'll merge it! :)

Thanks again!

[tcld]

I mentioned at least two changes that should probably be considered before merging this:

1. The version requirement for protobuf maybe should be changed to >=2.6.0 (assuming that syntax works) so that if somebody were to use Python 3, he could use whatever version works for him (same goes for Python 2, of course). Right now this patch sets protobuf to version 3.0.0a3 for all versions of Python, I think.

2. Properly handling range and xrange replacement so that Python 2 doesn't suffer from the change.

[psi29a]

the xrange to range is very important, xrange shouldn't have existed to begin with. ;)

[tcld]

Let's assume that any call from Python 2 to range() shall be replaced by a call to xrange(): Is it done globally somewhere? Or in every file that uses the function? A link on the topic would be handy, googling "python define" and similar doesn't help me. And I currently don't know how to reroute a call to a function like that.

[psi29a]

range (and xrange in python2) are keywords, they are built in to the python library by default and do not need to be included (include).

A simple search/replace of xrange/range is good enough.

We are not CPU bound on range so this isn't a problem.

FYI, there is no such thing as 'define' in Python like would be found in C/C++. You just create the function or object and then use it:

def foo(object):
     return 42

foo()

Python allows you to overwrite keywords, so you can create your own range if you wanted to. It would exist only in that scope though. This is generally NOT something you want to do. ;)

[tcld]

The search/replace is already done with this commit. Do you mean something dynamic that replaces the functions depending on which version of Python is installed? Sorry for the questions, but aside from defining a custom range() somewhere I wouldn't quite know how to do this.

[psi29a]

You don't need to do anything. :) range and xrange are keywords, I was just explaining that you can override them if you wanted. xrange is no longer a keyword in python >= 3, so it is better for the code that it is just 'range'. :)

[tcld]

Ok, then. I made another change to the protobuf-version, now 2.6.0 is a minimal requirement (but e.g. 3.0 still works nice for Python 3).

I hope that this doesn't cause new problems to crop up. Next step would be to take care of the unit-tests, I guess.

[psi29a]

I have good news and bad news: 18 passed, 22 failed

That is for python3.

Python2, there is no problem and everything works as expected.

You can try for yourself:

virtualenv venvpy3
source venvpy3/bin/activate
pip install -r requirements3.txt
pip install pytest
py.test -v tests

[psi29a]

cls = <class 'worldengine.biome.Biome'>, name = 'ocean'

    @classmethod
    def by_name(cls, name):
>       if name not in _BiomeMetaclass.biomes:
E       AttributeError: type object '_BiomeMetaclass' has no attribute 'biomes'

worldengine/biome.py:32: AttributeError

[tcld]

Thank you. That error is indeed correct, the class itself doesn't have that attribute (but any object of the class does). Python 2 is ok with that? I will take a look at this.

[psi29a]

I'm looking into it as well. :)

[psi29a]

There are a few issues, but the biggest issue is the use of metaclass... there is apparently problem with how Python 2.7 and 3 does this. I'm not sure how to make that work on both.

class Biome(object):
    __metaclass__ = _BiomeMetaclass

vs.

class Biome(object, metaclass = _BiomeMetaclass):

[ftomassetti]

stupid question: can't we use an if on the version and define the class either in one way or the other?

This seems better than my stupid comment: http://python-future.org/compatible_idioms.html#metaclasses

[psi29a]

We can, but it might be in our best interest to refactor this to be forward compatible without the cruft. :)

[tcld]

@ftomassetti That looks good, I'll try it out.

[psi29a]

Solved it with six. We'll likely be using six in the future anyway to bridge the difference/gap between 2 and 3.

[psi29a]

Please use my patch:

diff --git a/worldengine/biome.py b/worldengine/biome.py
index 8b05cdf..a2f2655 100644
--- a/worldengine/biome.py
+++ b/worldengine/biome.py
@@ -1,8 +1,8 @@
 """
 This file contains all possible Biome as separate classes.
 """
-
 import re
+from six import with_metaclass

 def _un_camelize(name):
@@ -23,9 +23,7 @@ class _BiomeMetaclass(type):
         return created_class

-class Biome(object):
-
-    __metaclass__ = _BiomeMetaclass
+class Biome(with_metaclass(_BiomeMetaclass, object)):

     @classmethod
     def by_name(cls, name):
@@ -35,7 +33,7 @@ class Biome(object):

     @classmethod
     def all_names(cls):
-        return _BiomeMetaclass.biomes.keys().sort()
+        return list(_BiomeMetaclass.biomes.keys()).sort()

-class Biome(object):
-
-    __metaclass__ = _BiomeMetaclass
+class Biome(with_metaclass(_BiomeMetaclass, object)):

     @classmethod
     def by_name(cls, name):
@@ -35,7 +33,7 @@ class Biome(object):

     @classmethod
     def all_names(cls):
-        return _BiomeMetaclass.biomes.keys().sort()
+        return list(_BiomeMetaclass.biomes.keys()).sort()

     @classmethod
     def name(cls):
@@ -211,7 +209,7 @@ class TropicalRainForest(Biome):
 # -------------

 def biome_name_to_index(biome_name):
-    names = _BiomeMetaclass.biomes.keys()
+    names = list(_BiomeMetaclass.biomes.keys())
     names.sort()
     for i in range(len(names)):
         if names[i] == biome_name:
@@ -220,7 +218,7 @@ def biome_name_to_index(biome_name):

 def biome_index_to_name(biome_index):
-    names = _BiomeMetaclass.biomes.keys()
+    names = list(_BiomeMetaclass.biomes.keys())
     names.sort()
     if biome_index < 0 or biome_index >= len(names):
         raise Exception("Not found")

[psi29a]

pip install six

please add that to requirements2.txt and requirements3.txt

[tcld]

Ok, worked for me too. A couple errors are still left, though. EDIT: Ok, thanks for the patch @psi29a .

[psi29a]

yup, looking into those too

[psi29a]

diff --git a/worldengine/common.py b/worldengine/common.py
index 0f90b80..b47c2ac 100644
--- a/worldengine/common.py
+++ b/worldengine/common.py
@@ -47,7 +47,7 @@ class Counter(object):

     def to_str(self):
         string = ""
-        keys = self.c.keys()
+        keys = list(self.c.keys())
         keys.sort()
         for w in keys:
             string += "%s : %i" % (w, self.c[w])

[tcld]

I put six into requirements_base, is that ok or should it be separated more clearly? six==1.10.0

And thanks for the patches. :)

PS: I will look out for the other sort()-problems myself, if it saves you some time.

[psi29a]

That's great, thanks! :)

try this one too:

diff --git a/tests/draw_test.py b/tests/draw_test.py
index 3e5bb9d..48080be 100644
--- a/tests/draw_test.py
+++ b/tests/draw_test.py
@@ -76,7 +76,7 @@ class TestDraw(TestBase):
         super(TestDraw, self).setUp()

     def test_biome_colors(self):
-        self.assertEqual(Biome.all_names(), _biome_colors.keys().sort())
+        self.assertEqual(Biome.all_names(), list(_biome_colors.keys()).sort())

     def test_elevation_color(self):
         for i in range(0, 20):

[tcld]

I had that already covered.^^

[psi29a]

and this? :)

diff --git a/worldengine/cli/main.py b/worldengine/cli/main.py
index bc252b7..73efdc6 100644
--- a/worldengine/cli/main.py
+++ b/worldengine/cli/main.py
@@ -137,7 +137,7 @@ def __get_last_byte__(filename):
             tmp_data = input_file.read(1024 * 1024)
             if tmp_data:
                 data = tmp_data
-    return ord(data[len(data) - 1])
+    return data[len(data) - 1]

[tcld]

Just looking at it. Is that safe? I don't understand why there is an integer to begin with. EDIT: Is it because of 'rb'? Then Python 2 should have complained, too, I think.

[psi29a]

Python 3 uses indexing as bytes object returns an integer, ord() here is redundant.

[tcld]

Yes, but won't this break Python 2's tests?

[psi29a]

Nope! :) As I said, it was redundant.

[tcld]

Oh, great. EDIT: 3 problems left.

[psi29a]

Can you uncomment the imex stuff and test that it works please?

[tcld]

Sure.

EDIT: @psi29a The tests seem fine with it. The problem (for Python 3) only occurs when running the installed version of worldengine.

FILE worldengine/cli/main.py", line 12, in from worldengine.imex import export ImportError: No module named 'worldengine.imex'

[tcld]

There is another discrepancy here: https://docs.python.org/3/library/pickle.html https://docs.python.org/2/library/pickle.html#pickle.load

Python 3 seems to assume that data is given in ASCII, the Python 2 version of load() cannnot take any arguments to control this behavior.

[tcld]

It seems that all problems that are left for me are

1. pickle.load()
2. the comparison of blessed pixels fails

EDIT: tests/drawing_functions_test.py::TestDrawingFunctions::test_draw_ancient_map_factor1 FAILED tests/drawing_functions_test.py::TestDrawingFunctions::test_draw_ancient_map_factor3 FAILED

[psi29a]

Yeah, pickle is shit. The good news is that we have protobuf that works for both python2 and 3 now thanks to you. :)

So we could drop pickle if we can't fin da good solution here.

[tcld]

Do I maybe have to regenerate the blessed images?

I think I'll push the current commit and leave the last two tests open for now. I don't have a good idea yet.

[psi29a]

I think that is a good idea. We've made progress.

[tcld]

@psi29a What should I do about imex/export.py? It certainly doesn't exist and prevents the compiled program from running.

[psi29a]

I don't understand, I see it there just fine.

take a peek in:

worldengine/imex/__init__.py

I have no problems running it.

[tcld]

Ok, I see the problem. Partially.

This is the import command: from worldengine.imex import export

which refers to the worldengine/imex/init.py which contains export().

I didn't even know that an init-file could contain functions to import. Maybe the method should be moved into its own file? I'll try to read a bit about that.

[psi29a]

Looks like you need to add six to .travis.yml

try this:

diff --git a/.travis.yml b/.travis.yml
index e100f6e..6206feb 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -10,6 +10,7 @@ cache:

 env:
     - TOX_ENV=py27
+    - TOX_ENV=py34
     - TOX_ENV=pypy
     - TOX_ENV=pyflakes
     - TOX_ENV=manifest
@@ -19,6 +20,7 @@ matrix:
   allow_failures:
     - os: osx
     - env: TOX_ENV=pypy
+    - env: TOX_ENV=py34
     - env: TOX_ENV=pyflakes
     - env: TOX_ENV=manifest

diff --git a/tox.ini b/tox.ini
index 39122a1..5b7deae 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,5 +1,5 @@
 [tox]
-envlist = py27, pypy, pyflakes, manifest
+envlist = py27, py34, pypy, pyflakes, manifest
 skip_install = True
 skipsdist = True

@@ -10,6 +10,7 @@ deps =
     noise
     nose
     protobuf
+    six

[tcld]

Thank you very much. Let's see what happens.

[psi29a]

That is how

__init__.py

is and works as intended. The module is the directory name and the functions/methods/objects are in there.

The original problem you had is somehow related to how it is installed and likely needs a tweak in setup.py, but isn't for this PR.

[tcld]

You seem to be right, the package was not (yet) listed in setup.py. I added it, which leads to a new error related to unicode.

[tcld]

The error stems from worldengine/protobuf/World_pb2.py line 309 since Python 3 doesn't have the unicode() function anymore. six, however, seems to offer some sort of replacement: https://pythonhosted.org/six/#six.text_type

Who generates World_pb2.py? Maybe this change is simple.

[tcld]

Yay, all checks passed. :) I'll look into that unicode() issue, however. Maybe it'll make all the tests run, too.