Yelp / MOE

A global, black box optimization engine for real world metric optimization.
Other
1.31k stars 140 forks source link

First step in converting to python 3.5 by running "2to3-3.5 -p -n -w ./moe". #459

Open printathing opened 8 years ago

printathing commented 8 years ago

Hi. This is my attempt to help https://github.com/Yelp/MOE/issues/415 by doing what I can to convert to python 3.5. I've used the 2to3-3.5 converter tool and pushed the result. I don't feel comfortable enough with the code to fix any manual issues that might need correction, but I'm hoping you can help out with that if you see so moved. You may want to make this your own branch and test first.

suntzu86 commented 8 years ago

I have minimal familiarity with python3, but this change seems to break python2:

E     File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/moe/bandit/bandit_interface.py", line 8
E       class BanditInterface(object, metaclass=ABCMeta):
E                                              ^
E   SyntaxError: invalid syntax

and many like it are amongst the errors from automated testing.

printathing commented 8 years ago

Most of these changes will likely break Python 2. Python 3 isn't backwards compatible as far as I understand. Someone will likely have to update the tests on this branch to use Python 3.5 On Apr 18, 2016 6:12 PM, "Eric Liu" notifications@github.com wrote:

I have minimal familiarity with python3, but this change seems to break python2:

E File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/moe/bandit/bandit_interface.py", line 8 E class BanditInterface(object, metaclass=ABCMeta): E ^ E SyntaxError: invalid syntax�[0m

and many like it are amongst the errors from automated testing.

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub https://github.com/Yelp/MOE/pull/459#issuecomment-211625739

suntzu86 commented 8 years ago

I don't think maintaining two separate branches is worth the effort... and I'm also not convinced now is the time to drop py2 support entirely.

I've seen the "six" package: https://pypi.python.org/pypi/six used at Yelp for writing code that runs in both >=py2.6 and py3. Also importing things like print from __future__. This seems to be pretty standard?

In some places that means you end up w/code that's a little uglier. Like how metaclasses are handled: http://python-future.org/compatible_idioms.html#metaclasses but I imagine once everyone is using py3, someone will write a code processor (if it isn't already a thing) to switch to pure py3 syntax.

ostrokach commented 8 years ago

Considering there have been only 2 commits to MOE in the last year and a half, maybe ditching Python 2 support would not the such a big deal. Writing code that is compatible with both Python 2 and 3 is super irksome...

printathing commented 8 years ago

With 2 commits in the last year and a half, maintaining 2 branches wouldn't be that big of a deal.. Changes would just be merged twice. Or maybe there's a git submodule of shared python 2 & 3-compatible code and the individual python2 and python3 branches use that git submodule. That way when a change is made once every 6 months, that person can decide to make it in python3 and port to python2 or can make it compatible with both and shared by both. I'm not one to like extra effort myself, but think of all of the effort that people who use MOE on python3 need to go through to get it to work. For all of my projects that use MOE, I now have to deliver 2 python environments each with their own specific packages - one for my python3 code, and one for a little ecosystem of MOE-compatible python2 that interfaces with it via command line. What a waste! Now, think of this effort multiplied by the hundreds of MOE users out there who would like to use python3. I don't want to volunteer anyone else's time, but I would like as a community for us to agree that this is worth pursuing together.

gokceneraslan commented 8 years ago

How about the future package?

gokceneraslan commented 8 years ago

Sent a new PR (https://github.com/Yelp/MOE/pull/466) which adds Python 3 support without breaking Python 2 support.