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

Use Cython to build extension modules #502

Merged
merged 3 commits into from
Jan 3, 2020
Merged

Conversation

jvkersch
Copy link
Contributor

@jvkersch jvkersch commented Dec 31, 2019

Removes the autogenerated .c files and relies on Cython being present at build time to build the extension modules.

Supersedes #459

Fixes #333

Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except for a minor question.

@@ -5,6 +5,7 @@ include chaco/tools/toolbars/images*.png
include chaco/tools/toolbars/images*.svg
include chaco/tools/toolbars/images*.txt
include chaco/layers/data/*.svg
recursive-include chaco *.pyx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am i missing something or should we be including the cntr.c file?
#459 was doing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C files that are listed in extension modules are included by default (https://packaging.python.org/guides/using-manifest-in/). I missed that in #459.

@rahulporuri
Copy link
Contributor

can you also update the PR description with a ref to the issue that this PR addresses?

@jvkersch
Copy link
Contributor Author

jvkersch commented Jan 1, 2020

Done and done. Thanks @rahulporuri for the quick review. Happy to enter a new decade of Python hacking with you 🎉 🎆

@jvkersch jvkersch merged commit 266c51d into master Jan 3, 2020
@jvkersch jvkersch deleted the dev/build-with-cython branch January 3, 2020 09:10
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.

Don't put Cythonized .c files in source control, but do ship in sdist
2 participants