mobiusklein / pyteomics.cythonize

Cython Implementations of Common Pyteomics Functions
Apache License 2.0
3 stars 2 forks source link

No .c files included, only Cython install possible #3

Closed levitsky closed 3 years ago

levitsky commented 3 years ago

setup.py has an install path for when Cython is not installed, relying on .c source files. Those files are not tracked in the repo, and are even ignored by .gitignore. As a result, installation fails when Cython is not available.

Should I add those .c files?

mobiusklein commented 3 years ago

The usual approach is to not add Cython-generated .c files into version control but to include them in any distributable archive (wheel, sdist, tarball). The (current) right way to deal with this would be to use a PEP518 build system specification, I suppose.

mobiusklein commented 3 years ago

Was the original situation that git clone followed by python setup.py install failed, or something else?

Sorry, I wasn't quite awake when I read this the first time through. If you're installing from PyPI, the package should already include the .c files (please correct me if this is wrong, major failure on my part then!). If you're installing from source with pip, then the PEP518 build system specification I just added should take care of installing Cython for you. If building with raw python setup.py install, this is still an issue though.

levitsky commented 3 years ago

Installing from Git source was indeed the issue. I made sure that Cython is not needed when installing from PyPI. The build system spec seems to be the right solution. Thank you!