Skip to content
This repository has been archived by the owner on May 25, 2022. It is now read-only.

Force the use of Cython's old build_ext command #261

Merged
merged 3 commits into from
Dec 8, 2016

Conversation

saimn
Copy link
Contributor

@saimn saimn commented Oct 27, 2016

A new build_ext command was added in Cython 0.25 (cython/cython#1456) but it breaks Astropy (I could not find why, see astropy/astropy#5437).

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. At least until we find why the new build_ext command does not work.

@eteq
Copy link
Member

eteq commented Oct 27, 2016

Thanks @saimn... This seemed fine to me by eye, but the tests are failing in a variety of convoluted ways... any idea why?

@saimn
Copy link
Contributor Author

saimn commented Oct 28, 2016

@eteq - No idea ... Locally tests pass on Py2, and on Py3 I had to use PYTHONDONTWRITEBYTECODE=1 to avoid "sandbox violation" errors with the __pycache__ directories, but I also get these errors on master.

@pllim
Copy link
Member

pllim commented Oct 28, 2016

That pyx failure is no more. So is this still relevant?

@MSeifert04
Copy link
Contributor

MSeifert04 commented Oct 28, 2016

@pllim I think so, because if I understand it correctly they just reverted to issue a deprecation warning:

C:\...\lib\site-packages\Cython\Distutils\old_build_ext.py:30: 
  UserWarning: Cython.Distutils.old_build_ext does not properly handle dependencies and is deprecated.
  "Cython.Distutils.old_build_ext does not properly handle dependencies "

@saimn
Copy link
Contributor Author

saimn commented Nov 2, 2016

@pllim - I think it is, as using the new build_ext breaks Astropy it is better to make sure that we always use the old one (until we find what is not working with the new).
However I have no idea why tests are failing ...

@saimn saimn closed this Nov 2, 2016
@saimn saimn reopened this Nov 2, 2016
@saimn
Copy link
Contributor Author

saimn commented Nov 2, 2016

Re-running the tests to make sure it was not something broken on Travis/Conda side.

@saimn
Copy link
Contributor Author

saimn commented Nov 2, 2016

And Travis now passes 😮
Appveyor fails because it tries to run tests for ci-helpers (ERROR collecting ci-helpers/test_env.py). Will fix that in appveyor.yml

@bsipocz bsipocz added this to the v1.3.0 milestone Dec 3, 2016
@bsipocz
Copy link
Member

bsipocz commented Dec 3, 2016

I think this will need a changelog entry. Could you add one @saimn, and then I can merge?

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.
Without this it tries to run tests from ci-helpers.
@saimn
Copy link
Contributor Author

saimn commented Dec 5, 2016

@bsipocz - Rebased and added changelog.

Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

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

Looks good to me too. Will merge!

@eteq eteq merged commit 7c21762 into astropy:master Dec 8, 2016
@saimn saimn deleted the cython-build_ext branch December 9, 2016 08:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants