Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cython generated c files should not be part of the source distribution #31

Open
ghisvail opened this issue Dec 29, 2013 · 7 comments
Open

Comments

@ghisvail
Copy link

From a development standpoint, any auto-generated files should not be generated as they can be refreshed at build time. Cython should be made a build dependency in the setup.py file and c-files should be excluded from MANIFEST.in.

From a packaging standpoint, the build system will be called twice, one for the Python 2 build and another one for Python 3. Each time, the c-files will be refreshed, overwriting the ones currently provided by the source distribution. This generates unnecessary pain for packaging.

@ghisvail
Copy link
Author

This is also a genuine reason why #25 is valid.

Feel free to have a check at my build setup in https:/ghisvail/pyNFFT, which was originally inspired from yours but corrected since then to comply with the Debian packaging guidelines.

@hgomersall
Copy link
Collaborator

This is an interesting point. The motivation for including the .c file is explicitly to remove the dependency on Cython. Is it possible to have two versions of the package - one for package builders and one for end users (the latter removing the dependency on Cython)? Do you think I should just ignore the dependency on Cython being a potential problem?

(I'm going to be slow replying to this issue until next week - I'm currently away from regular access to the web).

@ghisvail
Copy link
Author

Cython dependency is absolutely fine. It's mature enough (though it has not yet reached a 1.0 release) and well supported by the scientific community (Numpy, Scipy, scikit-learn...).

A rule of thumb is anything that can be built automatically should NOT be released as part of the source distribution, since it is not technically a source anymore but simply a residual from the build system.

Just make sure that cython is indeed listed in your install_requires in your setup and pip install should take care of the rest himself.

@grlee77
Copy link
Contributor

grlee77 commented Aug 24, 2017

Closing this as "won't fix". The decision was made to keep the Cython-generated .c files in the source distribution. see #182 (comment)

@jakirkham
Copy link
Member

Interestingly we wound up adding Cython as a build requirement anyways. ( #177 )

@hgomersall
Copy link
Collaborator

I think we should reopen this in light of the changes to allowing different library versions. Further discussion on the issue should be here, but the position IMO is that we should remove the C files.

@hgomersall hgomersall reopened this Mar 14, 2018
@grlee77
Copy link
Contributor

grlee77 commented Mar 14, 2018

I agree with removing them as well. The recent changes for build-time detection of the available precisions makes it necessary to generate them via Cython.

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

No branches or pull requests

4 participants