Check for Python 3 compatibility #25

Closed
mstimberg opened this Issue Mar 25, 2013 · 8 comments

Comments

Projects
None yet
2 participants
Owner

mstimberg commented Mar 25, 2013

Supporting Python 3 is not a high priority at the moment but if it is easy to do, there's no reason not to support it. Possibly, using 2to3 as part of the setup process is already enough:
http://python3porting.com/2to3.html#running-2to3-on-install

If yes, do this and add python 3 to the testing targets.

There are also a couple of uses of e.g. dict.items() that should be dict.iteritems(), etc.

@ghost ghost assigned mstimberg Mar 25, 2013

Owner

thesamovar commented Mar 25, 2013

I think this is a really good idea. We're going to have to deal with Python 3 compatibility at some point, and now that we're rewriting Brian from scratch it seems like it would be mad not to take the opportunity to do it now. I say let's get on this as soon as possible, and enforce writing code in a way that will work on both 2/3 where possible, or easily modifiable to 3 via 2to3 where not possible.

Owner

mstimberg commented Mar 25, 2013

I agree, I'll implement the 2to3 thing soon and then we can include it in the automatic testing. As far as I can see, it should not be much of an issue, now that we dropped Python < 2.6 support. We could try to write code that directly works with both Python 2 and 3, but I'm not sure it's worth the effort (we would need a lot of "from future import ..."). There are many things in the code that are translated very easily with 2to3 (e.g. "iteritems()" --> "items()", "xrange()" --> "range()", etc.). I think the only major thing we would have to take care of manually is to make the clear distinction between byte and unicode strings -- I guess basically all of the strings we have now (group names, etc.) should be unicode strings.

Owner

thesamovar commented Mar 25, 2013

We should probably add something to the developers guide with things to watch out for. I guess with 2to3 we develop always in 2.x and generate a 3.x version, right?

I haven't used 3 at all, so I don't know what the issues are with unicode strings, etc. They're the default in 3, right? If so, I guess we use them too unless we find a strong reason not to.

Owner

mstimberg commented Mar 25, 2013

We should probably add something to the developers guide with things to watch out for

You mean, something like the bottom of https://github.com/brian-team/brian2/blob/master/docs_sphinx/developer/guidelines/style.rst ;-) ?

I guess with 2to3 we develop always in 2.x and generate a 3.x version, right?

Exactly.

I haven't used 3 at all, so I don't know what the issues are with unicode strings, etc. They're the default in 3, right? If so, I guess we use them too unless we find a strong reason not to.

Yes, unicode is default in 3, whereas byte strings are default in 2, so we'll have to write u"string" in python 2. The distinction is messy in python 2 (and that was one of the main reasons for python 3 in the first place), you can use them interchangeably very often whereas python 3 makes a clear difference. But the only case where this is an issue for us will be when writing to/reading from files, I think.

Owner

mstimberg commented Mar 26, 2013

Ok, I started a branch and pull request #26 for this issue. There was a slight complication that I did not anticipate: For the "write the default preferences to a file" mechanism we have to import brian2 in setup.py, which obviously fails for Python3 because setup.py is run before the 2to3 fixer converted Python code from 2 to 3. The solution is not that complicated though: We overwrite the install_data command to insert the installation directory in the path so that it imports brian2 from there. This seems to work fine, but there are some issues in our code that 2to3 cannot deal with currently.
One example (I'll add them to the developer docs soon): 2to3 automatically converts operator.isNumberType(obj) into isinstance(obj, numbers.Number), but in the units system we do from operator import isNumberType and therefore this replacement does not work. But in this particular case, we can use the isinstance variant anyway, since it works since Python 2.6

Owner

thesamovar commented Mar 26, 2013

I never liked isNumberType anyway. ;)

Owner

mstimberg commented Mar 26, 2013

Hah, so it turns out that the above example is a good one because it shows the limitations of 2to3... operator.isNumberType returns True for a numpy array but isinstance(obj, numbers.Number) does not... This shows how important it is to have a test suite with good coverage!

Owner

mstimberg commented Mar 29, 2013

Closed with the merge of #26 (commit 6c3a160).

@mstimberg mstimberg closed this Mar 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment