Fix building for Python 3.8 #765
Conversation
|
Do the compiler warnings go away with Cython 0.29.14? If so, can we just set that to the minimum version and not have to worry about disabling deprecation warnings? |
|
For reference, I'm using Python 3.8.0 on macOS and Linux built from the source tarball by pyenv, and on Windows Python 3.8.0 from the Anaconda distribution. The warnings are present only on macOS and only with Cython 0.29.14. For reference, these are the warnings I get: The relevant lines from which is basically repeated for each These warnings are not present on Windows or on Linux for Cython >=0.29.12. With Cython < 0.29.12, the compile of Interestingly, Cython 0.29.12 on Windows and 0.29.13 on macOS do not generate the warnings that cython/cython#3171 and cython/cython#3201 were merged to fix. In fact, they don't generate any warnings at all (except the standard NumPy one). 🤷♂ Basically, I have no idea what's going on here. The changes in this PR allow building with Python 3.8.0 without spurious warnings. That's all I know 😅 |
SConstruct
Outdated
| elif python_version >= LooseVersion("3.8"): | ||
| # Reset the minimum Cython version if the Python version is 3.8 or higher | ||
| # Due to internal changes in the CPython API, more recent versions of | ||
| # Cython are necessary to build for Python 3.8 | ||
| cython_min_version = LooseVersion("0.29.12") |
There was a problem hiding this comment.
I think it would be helpful for the comments here to be slightly more clear that there's nothing Cantera-specific about any of this: Only Cython >= 0.29.12 is compatible with Python 3.8, and we're just setting this dependency so that users get a more useful error message than the compilation errors that would otherwise result.
interfaces/cython/SConscript
Outdated
| # Only applies to Python 3.8 and Cython < 3.0. The source of the warnings | ||
| # will be removed after Python 3.8, so the minimum version of Cython will | ||
| # need to change for Python 3.9. | ||
| if LooseVersion(py_version) == LooseVersion("3.8"): |
There was a problem hiding this comment.
Given the documentation for distutils.sysconfig.get_python_version(), this could just be:
| if LooseVersion(py_version) == LooseVersion("3.8"): | |
| if py_version == "3.8": |
Due to internal C API changes in CPython, newer versions of Cython are required to compile with Python 3.8 and higher.
The clang compiler on macOS prints a number of warnings about deprecated fields in the CPython 3.8 API. These warnings are spurious because they are caused by Cython and will be handled in a future release for Python 3.9. They also appear to only occur on macOS.
bryanwweber
force-pushed
the
fix-py-38
branch
from
50d9ce1
to
484441a
Compare
|
For what it's worth, there were various other internal changes in CPython in the beta period for 3.8, such that the Cython maintainer states that Cython 0.29.12 is the first version compatible with the released CPython 3.8: https://bugs.python.org/issue37221#msg347468 Also related, with more information for future researchers: python/cpython#14193 and https://bugs.python.org/issue37250 and cython/cython#2976 |
Codecov Report
@@ Coverage Diff @@
## master #765 +/- ##
=========================================
+ Coverage 71.4% 71.4% +<.01%
=========================================
Files 372 372
Lines 43859 43859
=========================================
+ Hits 31316 31317 +1
+ Misses 12543 12542 -1
Continue to review full report at Codecov.
|
Thanks for contributing code! Please include a description of your change and
check your PR against the list below (for further questions, refer to the
contributing guide).
scons build&scons test) and unit tests address code coverageChanges proposed in this pull request
_cantera.cppthat are caused by a deprecated feature that Cython handles