WillAyd commented on code in PR #359:
URL: https://github.com/apache/arrow-nanoarrow/pull/359#discussion_r1452462096
##########
.github/workflows/python.yaml:
##########
@@ -36,7 +36,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
- python-version: ['3.10']
+ python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
Review Comment:
Much of the scientific python community has already dropped Python 3.8
support per NEP 29:
https://numpy.org/neps/nep-0029-deprecation_policy.html
Up to you of course what to support, but maybe not worth going as far back
as 3.8 (which is EOL this year for Python as well)
##########
python/setup.py:
##########
@@ -65,6 +66,11 @@ def get_version(pkg_path):
extra_link_args = []
extra_define_macros = []
+# Setting the CFLAGS environment variable doesn't seem sufficient in all cases
to add
Review Comment:
Do you know where this limitation was coming into play? I've generally
always had success with CFLAGS, unless activating a conda environment with a
compilers package that may overwrite this. I don't _think_ that is the problem
here but figured I'd mention
##########
.github/workflows/verify.yaml:
##########
@@ -138,7 +138,9 @@ jobs:
- {
platform: "centos7",
arch: "amd64",
- compose_args: "-e NANOARROW_ACCEPT_IMPORT_GPG_KEYS_ERROR=1"
+ # Currently the Python on the centos7 image is 3.6, which does
not support
Review Comment:
Instead of using a base centos 7 you might also want to just use the same
centos image that cibuildwheel uses for python packaging. That has quite a few
versions of python installed on it
quay.io/pypa/manylinux2014_x86_64
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
