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

Add Cython dependency #118

Merged
merged 1 commit into from
Oct 30, 2015
Merged

Add Cython dependency #118

merged 1 commit into from
Oct 30, 2015

Conversation

bsmithyman
Copy link
Member

This should really be dependent on Cython, if we're going to call cythonize directly in the build script, and if Cython is needed for good performance. Or, if not, we would need to make the import of cythonize conditional, and then skip building the extension if Cython isn't installed. Alternatively, don't build the .pyx file, and check in a compiled .c file; then make the following changes to setup.py:

  • replace from Cython.Build import cythonize on Line 10 with from setuptools.extension import Extension
  • replace ext_modules = cythonize('SimPEG/Utils/interputils_cython.pyx') on Line 53 with ext_modules = [Extension('interputils_cython', sources=['SimPEG/Utils/interputils_cython.c'])]

@rowanc1
Copy link
Member

rowanc1 commented Oct 28, 2015

Do you know what the standard in the community is wrt checking in the compiled code?

@bsmithyman
Copy link
Member Author

Yeah, good question, I'm not sure. To be fair, it's not compiled to binary code, it's compiled/templated to C, so it should still sit in the Git repository cleanly, just as a derived file. Kind of like checking in compiled documentation, vs. doxygen source. In principle, it's all cpython calls, so it should be portable across cpython installations (but not, for example, pypy or jython). It would add a step to generating the source if you change the pyx file, but other than that I suppose there's no real disadvantage. Perhaps there's a way to have it auto-generated with a Git hook? Also, it could be done only on releases (to lose the Cython dependency w/ pip installations), and keep the original in the Git tree. We could write a "make_release" script to handle duplicating things to a .gitignore'd subdirectory and changing the setup file. All just thoughts, though.

rowanc1 added a commit that referenced this pull request Oct 30, 2015
@rowanc1 rowanc1 merged commit fc3590e into master Oct 30, 2015
@rowanc1 rowanc1 deleted the bsmithyman-cythondep branch October 30, 2015 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants