Skip to content

Conversation

@grlee77
Copy link
Contributor

@grlee77 grlee77 commented Nov 19, 2018

We do not currently specify the Cython language_level. This results in a warning from Cython 0.29 that the default is going to change from language_level=2 to language_level=3str in the future.

As we are dropping Python 2 support in the next release, I thought we should just explicitly specify language_level=3. This PR adds the language_level directive at the top of each .pyx file.

There was one place where / needed to be // to maintain integer division. The rest of the changes are to the import statements and were necessary to get the files to compile under this language level.

This is built on top of #434 so that the Python 2.7 test cases wouldn't be run. Only the c7c9c2f commit is new to this PR.

tacaswell added a commit to tacaswell/pywt that referenced this pull request Feb 4, 2019
This is a simpler alternative to PyWavelets#435
@grlee77 grlee77 force-pushed the cython_language_level branch from a20d27e to cf492ae Compare February 4, 2019 19:43
@grlee77
Copy link
Contributor Author

grlee77 commented Feb 4, 2019

I rebased this on current master to get rid of all the already merged commits from #434. I then moved the language_level statement to setup.py as suggested by @tacaswell in #451 since it is applied globally.

@grlee77
Copy link
Contributor Author

grlee77 commented Feb 4, 2019

The only change here to the Cython code aside from import syntax was the need to specify integer division at the following location:
https://github.com/PyWavelets/pywt/pull/435/files#diff-c8a5633b1197b648cc4d0c0949ed195eL1047

@rgommers rgommers merged commit f2f448f into PyWavelets:master Feb 4, 2019
@rgommers
Copy link
Member

rgommers commented Feb 4, 2019

LGTM, merged. Thanks @grlee77

@rgommers rgommers added this to the v1.1 milestone Feb 4, 2019
@grlee77
Copy link
Contributor Author

grlee77 commented Feb 4, 2019

I got a notification about failures on ReadtheDocs and will look into it. We probably just need to make sure the Cython version being used there is recent enough.

https://readthedocs.org/projects/pywavelets/builds/8509525/

@rgommers
Copy link
Member

rgommers commented Feb 4, 2019

Ah yes, we're still pinning to Cython 0.20.2 in util/readthedocs/requirements.txt

@grlee77
Copy link
Contributor Author

grlee77 commented Feb 4, 2019

#452 should fix the documentation build

grlee77 pushed a commit to grlee77/pywt that referenced this pull request Feb 10, 2019
@grlee77 grlee77 deleted the cython_language_level branch November 13, 2019 00:51
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.

2 participants