Skip to content
This repository was archived by the owner on Nov 11, 2024. It is now read-only.

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

Merged
eteq merged 3 commits into
astropy:masterfrom
saimn:cython-build_ext
Dec 8, 2016
Merged

Force the use of Cython's old build_ext command#261
eteq merged 3 commits into
astropy:masterfrom
saimn:cython-build_ext

Conversation

@saimn

@saimn saimn commented Oct 27, 2016

Copy link
Copy Markdown
Contributor

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

eteq commented Oct 27, 2016

Copy link
Copy Markdown
Member

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

@saimn

saimn commented Oct 28, 2016

Copy link
Copy Markdown
Contributor Author

@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

pllim commented Oct 28, 2016

Copy link
Copy Markdown
Member

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

@MSeifert04

MSeifert04 commented Oct 28, 2016 •
edited
Loading

Copy link
Copy Markdown
Contributor

@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

saimn commented Nov 2, 2016

Copy link
Copy Markdown
Contributor Author

@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

saimn commented Nov 2, 2016

Copy link
Copy Markdown
Contributor Author

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

@saimn

saimn commented Nov 2, 2016

Copy link
Copy Markdown
Contributor Author

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 mentioned this pull request Nov 23, 2016
Closed
@bsipocz bsipocz added this to the v1.3.0 milestone Dec 3, 2016
@bsipocz

bsipocz commented Dec 3, 2016

Copy link
Copy Markdown
Member

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

saimn added 2 commits December 4, 2016 22:42
@saimn
Force the use of Cython's old build_ext command
6b4439f 
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
Run only astropy_helpers tests on appveyor
14c5646 
Without this it tries to run tests from ci-helpers.
@saimn

saimn commented Dec 5, 2016

Copy link
Copy Markdown
Contributor Author

@bsipocz - Rebased and added changelog.

eteq
eteq approved these changes Dec 8, 2016
View reviewed changes

@eteq eteq left a comment

Copy link
Copy Markdown
Member

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.

Reviewers

2 more reviewers
@astrofrog astrofrog astrofrog approved these changes
@eteq eteq eteq approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v1.3.0

Development

Successfully merging this pull request may close these issues.

6 participants

@saimn @eteq @pllim @MSeifert04 @bsipocz @astrofrog