[#1237] Update SciPy to 1.9.3 #1359
Conversation
|
Have you tested your builds with the pkgtest app as described in "Testing a package" in the README? If so, which Python versions and ABIs have you checked? |
| {% if PY_VER == "3.8" %} | ||
| {% set numpy_version = "1.19.5" %} | ||
| {% elif PY_VER in ["3.9", "3.10", "3.11"] %} | ||
| {% set numpy_version = "1.23.3" %} | ||
| {% endif %} |
There was a problem hiding this comment.
There's no need for this, because we aren't building new packages for Python 3.8 anymore.
| + /* Chaquopy: Prevent C/C++ macro collision */ | ||
| + #ifndef __cplusplus | ||
| + #ifndef isnan |
There was a problem hiding this comment.
Why is this patch necessary – what error message do you get without it?
And why is __cplusplus the appropriate way of doing it? Is this file included in any C++ code? If not, then the __cplusplus check is just equivalent to disabling the code completely.
If the patch is necessary, then the inner #ifndef should be left at its original indentation to make the patch easier to read.
|
|
||
| from scipy._build_utils import numpy_nodepr_api | ||
|
|
||
| - |
There was a problem hiding this comment.
This change appears to be insignificant.
| -cdef void _visit_voronoi(qhT *_qh, void *ptr, vertexT *vertex, vertexT *vertexA, | ||
| +cdef void _visit_voronoi(qhT *_qh, FILE *ptr, vertexT *vertex, vertexT *vertexA, |
There was a problem hiding this comment.
Please add a "Chaquopy: " comment saying that you changed void to FILE, and explaining why that was necessary.
| -import numpy | ||
| - | ||
| +# Chaquopy: skip "import numpy" |
There was a problem hiding this comment.
Was there some reason to change this? The previous version was less disruptive, as it retained the two lines after the imports.
| codelens = [len(x) for x in all_codes] | ||
| - last = numpy.prod(codelens) - 1 | ||
| + | ||
| + |
There was a problem hiding this comment.
Please avoid insignificant whitespace changes.
| + # Chaquopy: Force return | ||
| + return |
There was a problem hiding this comment.
This says what you're doing, but it doesn't explain why. If the rest of the function would cause some problem, then include a comment saying what it was.
| +os.environ["FC"] = os.path.join(os.path.expanduser("~"), "Downloads/gcc-arm64-linux-x86_64/aarch64-linux-android-4.9/bin/aarch64-linux-android-gfortran") | ||
| +os.environ["F77"] = os.path.join(os.path.expanduser("~"), "Downloads/gcc-arm64-linux-x86_64/aarch64-linux-android-4.9/bin/aarch64-linux-android-gfortran") | ||
| +os.environ["F90"] = os.path.join(os.path.expanduser("~"), "Downloads/gcc-arm64-linux-x86_64/aarch64-linux-android-4.9/bin/aarch64-linux-android-gfortran") |
There was a problem hiding this comment.
One thing to note is that I had to hardcode some paths for Fortran in SciPy's setup.py file (despite seemingly placing the Fortran library for Android in the proper folder).
The proper folder is not ~/Downloads, but chaquopy/server/pypi/fortran, as it says in pypi/README.md. build-wheel.py sets these environment variables to point into that location.
|
If you want to proceed with this, please respond to my previous comments, and I'll reopen the PR. |
|
Actually, I'll leave this open for future reference, since it does contain some potentially useful patch updates. |
This updates SciPy to 1.9.3 which should provide support for Python 3.8 through 3.11. I did my testing with Python 3.11.
One thing to note is that I had to hardcode some paths for Fortran in SciPy's setup.py file (despite seemingly placing the Fortran library for Android in the proper folder). If anyone knows how to avoid hardcording the path for Fortran, I can update the patch file!
Closes #1237