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

Replace old build_ext with new cythonize(...) version. #1456

Merged
merged 6 commits into from Sep 10, 2016

Conversation

robertwb
Copy link
Contributor

@robertwb robertwb commented Sep 7, 2016

Rather than redirect everyone to the one in Cython.Build, simply upgrade the Cython.Distutils version. In particular, this is the version picked up by setuptools.

Notably, however, the new one takes far fewer options (this configuration primarily moved into in-file directives). The old one is kept as Cython.Distutils.old_build_ext to ease migration; we should consider which, if any, additional options should be moved to the new one.

@scoder
Copy link
Contributor

scoder commented Sep 10, 2016

We'll need an extended test cycle for the 0.25 release anyway, so let's get this in and fix it in master.

@scoder scoder merged commit 849d829 into cython:master Sep 10, 2016
@jdemeyer
Copy link
Contributor

This breaks https://github.com/zeromq/pyzmq

@jdemeyer
Copy link
Contributor

In order for this to be a viable replacement to cythonize(), it should support passing keywords to cythonize. For example, Sage passes the keywords nthreads, build_dir, force, aliases, compiler_directives.

@robertwb
Copy link
Contributor Author

I agree that this is not a replacement for calling a customized cythonize(), but should be a step forward. For better or for worse, the distutils way to customize thing is to subclass (e.g. to specifying the build_ext, it expects a class, not an instance).

@robertwb
Copy link
Contributor Author

And I'd want to think long and hard about what command line options to add, given that they're even harder to change.

@jdemeyer
Copy link
Contributor

I agree that this is not a replacement for calling a customized cythonize(), but should be a step forward.

And it serves as nice "documentation" on how to customize the build_ext command.

@messense
Copy link

breaks pyyaml. See #1498

saimn added a commit to saimn/astropy-helpers that referenced this pull request Oct 27, 2016
A new build_ext command was added in Cython 0.25 but it breaks Astropy.
cython/cython#1456

Cython 0.25.1 was released to revert this change:
cython/cython@4ecdd3e

This commit ensures that we always use the old build_ext, whatever the
Cython version is.
saimn added a commit to saimn/astropy-helpers that referenced this pull request Dec 4, 2016
A new build_ext command was added in Cython 0.25 but it breaks Astropy.
cython/cython#1456

Cython 0.25.1 was released to revert this change:
cython/cython@4ecdd3e

This commit ensures that we always use the old build_ext, whatever the
Cython version is.
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

4 participants