Skip to content

Conversation

@astrofrog
Copy link
Member

@astrofrog astrofrog commented Nov 23, 2016

Because of #5319, we now require Cython 0.21 or later, so this PR updates the minimum required version and also fixes the CircleCI build.

@bsipocz
Copy link
Member

bsipocz commented Nov 23, 2016

Could you please also add PYTEST_HEADER_MODULES['cython'] = 'cython' to astropy/conftest.py? Or maybe even to astropy/tests/pytest_plugins.py, but I think it would be actually better to clean up the latter a bit as all the affiliates inherit that dictionary.

@astrofrog
Copy link
Member Author

@bsipocz - done

@MSeifert04
Copy link
Contributor

MSeifert04 commented Nov 23, 2016

Not sure but there is also a pip-requirements and pip-requirements-dev file, would it make sense to update these as well?

Or use these instead of updating the narrative docs?

@astrofrog
Copy link
Member Author

@MSeifert04 - done (we should still update the narrative docs)

@saimn
Copy link
Contributor

saimn commented Nov 23, 2016

It seems posix.types was added in Cython 0.21: cython/cython@4345655
If it is useful to keep compatibility with previous versions, it is easy to avoid using posix.types. Otherwise thanks for updating the requirements !

@astrofrog
Copy link
Member Author

@saimn - if it is indeed easy to avoid using posix.types, that would be a preferable fix, just to avoid reports of this issue when people accidentally use versions that are too old. Could you do a PR for this?

@eteq
Copy link
Member

eteq commented Nov 23, 2016

👍 - looks like the remaining CircleCI fault is to be fixed in #5497 ?

@astrofrog
Copy link
Member Author

@eteq - yes, that's correct!

@eteq
Copy link
Member

eteq commented Nov 24, 2016

Alright, this looks fine aside from the other CircleCI issue, so will go ahead and merge.

@eteq eteq merged commit 529c64a into astropy:master Nov 24, 2016
@eteq
Copy link
Member

eteq commented Nov 24, 2016

One follow-up question (which is only relevant for the milestone, not the code): do we consider it OK to change the Cython requirement as part of a bugfix release? That is, this is currently milestoned for 1.0.11, but should it instead be 1.3?

@saimn
Copy link
Contributor

saimn commented Nov 24, 2016

@eteq @astrofrog : see #5501 to avoid changing the Cython requirement for 1.0.11

eteq added a commit that referenced this pull request Dec 21, 2016
eteq added a commit that referenced this pull request Dec 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants