Doesn't build / install properly with Python 3

[wackywendell]

Right now, Python 3 sees the setup.py and thinks there are two modules: pyvoro and voroplusplus, and installs them separately. Unfortunately, this does not work with relative imports.

I forked this repository, and updated it to fix that issue here.

In the process of troubleshooting, I also converted it to use Cython.Build.cythonize in the setup.py build, so I am submitting this as an "Issue" rather than a "Pull Request", as my fix does remove some install functionality. If you do want that, I'm happy to submit a pull request...

[joe-jordan]

This is a Python 2.X module, it doesn't need to compile with Python 3. You mention testing in 2.7 and 2.4 in your pull request, could you confirm that this is actually broken to start with in these versions, and specify which platform?

As I mentioned in the reply to your line comment, Cython cannot be a compile requirement without giving very very confusing errors to many many users who have old versions.

As for newer versions, anyone is free to use command line Cython to update the voroplusplus.cpp file to a newer version, provided it still works - if you do that and send me a pull request (for that only,) I'd be happy to accept it.

[wackywendell]

Hi,

On Python 3, this compiles (python setup.py build) and installs (python install) just fine, but then when importing you get a confusing error:

$ python -c "import pyvoro"                                
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/home/wendell/.local/lib/python3.4/site-packages/pyvoro/__init__.py", line 1, in <module>
    import voroplusplus
  File "voroplusplus.pyx", line 36, in init pyvoro.voroplusplus (pyvoro/voroplusplus.cpp:2696)
SystemError: Parent module '' not loaded, cannot perform relative import

It turns out that this is because voroplusplus is not built as part of the pyvoro module:

$ ls ~/.local/lib/python3.4/site-packages/
...
pyvoro
pyvoro-1.3.2-py3.4.egg-info
voroplusplus.cpython-34m.so

By telling setup.py you want voroplusplus to be a submodule of pyvoro, and then having pyvoro/__init__.py using a "relative import" of voroplusplus, this is solved.

I'll remove the .gitignore stuff too, sorry!

[joe-jordan]

...I'm confused, surely pure python 2 code doesn't run under python 3 anyway! Have I just accidentally used no python3-incompatible syntax? This is fluke, if so, and you definitely shouldn't rely on it in future versions!

[joe-jordan]

This is another breaking change between setup.py syntax between python 2 and 3, it seems, in which case we don't need a fix for it for python 2. Sorry, but I'm leaning towards marking this as WONTFIX.

[wackywendell]

Ah, you're right! It does fine on Python 2 and installs as a separate module.

However, the syntax I included works just fine on Python 2, and is clearer: it specifically shows (in both places) that voroplusplus is a submodule of pyvoro. You don't have to accept, though.

On the other issue (Python 3 support), you bring up a good issue: what about Python 3 support? Why don't you want to support Python 3? I can understand not dropping Python 2, but I don't think there's any reason not to attempt supporting Python 3, now that its the future of Python. The big changes of Python 3 are the str, unicode -> str change and the print becoming a function change, neither of which should affect this much.

I'd really like Python 3 support. Do you want me to try and maintain a Python 3 version of this?

[wackywendell]

Also, note that you can have setuptools automatically run 2to3 on the code when it builds; see here.

[joe-jordan]

Most scientists are still using Python 2, and will be for the forseeable future. There isn't any reason per se for excluding support, but I literally have no idea if the code works properly in Python 3.

If you are familiar enough with the syntax of 3, and are happy to audit the code for me, suggesting compatibility changes, then please go ahead. I'll be happy to accept pull requests that solve one python 3 issue at a time, and I guess installation would be the first.

However, could you please use your forked version under python 3 and check that there are no other syntax errors once the relative import is fixed? I would then also need an assessment of everywhere integer division is used in the code (its the kind of thing I use in all sorts of places to configure things - I know for a fact that the old build system used some things like that to generate build folder names, for example.)

Once we're happy there are unlikely to be any problems, we'll need unit tests that cover every code path, which both give the same answer in both versions. Then, and only then, will I be happy putting up the code on PyPI as compatible with both pip 2 and 3!

(note - 2to3 really can't know whether variables hold integers or floats, so I don't trust it at all.)

[joe-jordan]

I don't mean to sound, well, OTT about all this, but I don't have time to work on it myself at the moment (writing up the PhD!) and I wouldn't want there to be a broken version out there, as I would get lots of bug reports (where lots means more than 1), which I don't have time to act on at the moment.

[wackywendell]

I just went through the code, and there's only one place division is used as integer division. If I'm not mistaken, other than setup.py, there are only 321 lines to check (__init__.py and voroplusplus.pyx); the .cpp code is not sensitive to Python version.

I'll work on some unit tests. I understand you're busy.

Good luck with the thesis writing! Out of curiosity, what are your plans afterwards? I'm in my 5th year, but for various reasons not graduating for a bit... so I'm just curious.

[wackywendell]

For posterity, I'm currently working on said Python 3 branch here.

[joe-jordan]

Thanks for your work on this. Do keep me up to date here with progress/issues - I may not respond but I will read it!

My uni has a 4 year limit, which I'm 2 months away from! so writing frantically, and plotting graphs and trying to think of conclusions during LaTeX compilation.... I don't have any plans for afterwards yet, except earn some money and be able to replace my (very broken) Mac!

[wackywendell]

Ah, I see! Good luck! I'll keep you posted!