mittinatten / freesasa

C-library for calculating Solvent Accessible Surface Areas
http://freesasa.github.io/
MIT License
105 stars 37 forks source link

Python3 & Cython3 #20

Closed vlado-s closed 7 years ago

vlado-s commented 7 years ago

Hello, is it possible to build the Python bindings for python3.x using cython3.x? Configure accepts the option --with-python=python3, but it does not recognize cython3 (mixing with cython2 results in failure, as expected). Thank you for a reply.

mittinatten commented 7 years ago

I haven't tried this, but I can see what I can do. I have never used Python3, so I don't know about compatibility. But if the module itself is valid Python3, perhaps it could be solved by adding a configure option --with-cython. If you have any expertise here I would be happy for any input or a PR.

mittinatten commented 7 years ago

I tried with Python 3.6 and Cython 0.25.2 on my Mac and just had to add one line to make it work (see the second commit to the dev-branch above). I use macports and had to change the global to cython to py36-cython to make it work. When I changed back to Python 2.7 it still worked without changing cython back, so there seems to be backwards compatibility here. Does it work for you as well?

vlado-s commented 7 years ago

Thanks'. Where exactly do you change the global to py36-cython? Is that something Mac specific? For now I just hard coded at line 6080 in the configure script: ac_cv_prog_CYTHOn="cython3" The configure script seems to accept that but in make I get a problem in freesasa.pyx at the line 509 (line number after making the change you proposed above). The line reads: cdef const double *coord = freesasa_structure_coord_array(self._c_structure);

The complete error message is: freesasa.pyx:509:88: Syntax error in C variable declaration Traceback (most recent call last): File "setup.py", line 26, in ext_modules = cythonize(extensions) File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 798, in cythonize cythonize_one(*args[1:]) File "/usr/lib/python3/dist-packages/Cython/Build/Dependencies.py", line 915, in cythonize_one raise CompileError(None, pyx_file) Cython.Compiler.Errors.CompileError: freesasa.pyx

Anyway, I try to use it as it worked with older python & cython versions. Sorry for the trouble.

mittinatten commented 7 years ago

Yes, it's a macports specific thing.

The configure script just checks if cython is present, however, and doesn't control which cython is used when building the module. So your change to the configure script shouldn't have any effect on the result. The module is built by calling cython from setup.py using the function cythonize(), so it uses whatever version of Cython is associated with the active version of Python. I must admit I don't know much about the intricacies here, and I'm not sure how to best deal with several versions of Python on the same machine, I suspect it is platform-dependent.

I see that there's a semi-colon at the end of the line that causes en error for you, I added a few of those without thinking apparently (the context switching between C and Python is apparently difficult :) ). It could be that you have a version or configuration of Cython that is more sensitive to errors than mine. I committed a version where I removed all semicolons (they really shouldn't be there). Does that help?

mittinatten commented 7 years ago

Closing this for now, discussion continues in #23.