Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-36730: [Python] Add support for Cython 3.0.0 #36745

Closed
wants to merge 12 commits into from
Closed

GH-36730: [Python] Add support for Cython 3.0.0 #36745

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6a6171a
GH-36730: [Python] Add support for Cython 3.0.0
kou Jul 18, 2023
ecad6bd
Add missing const
kou Jul 18, 2023
945ac7d
Suppress C4551
kou Jul 18, 2023
978b47c
Follow generator change
kou Jul 19, 2023
cb6f36d
Unpin Cython
kou Jul 19, 2023
7d9a1ed
Add a missing :
kou Jul 19, 2023
49b2e25
Add missing NULL check
kou Jul 20, 2023
3ae02ee
Don't use StopIteration in __iter__
kou Jul 20, 2023
e76f77a
Use __next__
kou Jul 20, 2023
113f90c
Don't use __next__ for MapScalar
kou Jul 20, 2023
001f562
Fix table format
kou Jul 20, 2023
3e83b00
Try
kou Jul 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ jobs:
shell: bash
run: |
gem install test-unit
pip install "cython<3" setuptools six pytest jira
pip install cython setuptools six pytest jira
- name: Run Release Test
env:
ARROW_GITHUB_API_TOKEN: ${{ secrets.GITHUB_TOKEN }}
Expand Down
2 changes: 1 addition & 1 deletion ci/conda_env_python.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# don't add pandas here, because it is not a mandatory test dependency
boto3 # not a direct dependency of s3fs, but needed for our s3fs fixture
cffi
cython<3
cython
cloudpickle
fsspec
hypothesis
Expand Down
2 changes: 1 addition & 1 deletion dev/release/verify-release-candidate.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,7 +665,7 @@ test_python() {
show_header "Build and test Python libraries"

# Build and test Python
maybe_setup_virtualenv "cython<3" numpy setuptools_scm setuptools || exit 1
maybe_setup_virtualenv cython numpy setuptools_scm setuptools || exit 1
maybe_setup_conda --file ci/conda_env_python.txt || exit 1

if [ "${USE_CONDA}" -gt 0 ]; then
Expand Down
29 changes: 18 additions & 11 deletions python/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,37 +168,44 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${PYARROW_CXXFLAGS}")

if(MSVC)
# MSVC version of -Wno-return-type-c-linkage
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4190")
string(APPEND CMAKE_CXX_FLAGS " /wd4190")

# Cython generates some bitshift expressions that MSVC does not like in
# __Pyx_PyFloat_DivideObjC
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4293")
string(APPEND CMAKE_CXX_FLAGS " /wd4293")

# Converting to/from C++ bool is pretty wonky in Cython. The C4800 warning
# seem harmless, and probably not worth the effort of working around it
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4800")
string(APPEND CMAKE_CXX_FLAGS " /wd4800")

# See https://github.com/cython/cython/issues/2731. Change introduced in
# Cython 0.29.1 causes "unsafe use of type 'bool' in operation"
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /wd4804")
string(APPEND CMAKE_CXX_FLAGS " /wd4804")

# See https://github.com/cython/cython/issues/4445.
#
# Cython 3 emits "(void)__Pyx_PyObject_CallMethod0;" to suppress a
# "unused function" warning but the code emits another "function
# call missing argument list" warning.
string(APPEND CMAKE_CXX_FLAGS " /wd4551")
else()
# Enable perf and other tools to work properly
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
string(APPEND CMAKE_CXX_FLAGS " -fno-omit-frame-pointer")

# Suppress Cython warnings
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-unused-variable -Wno-maybe-uninitialized")
string(APPEND CMAKE_CXX_FLAGS " -Wno-unused-variable -Wno-maybe-uninitialized")

if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID STREQUAL
"Clang")
# Cython warnings in clang
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-parentheses-equality")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-constant-logical-operand")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-missing-declarations")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-sometimes-uninitialized")
string(APPEND CMAKE_CXX_FLAGS " -Wno-parentheses-equality")
string(APPEND CMAKE_CXX_FLAGS " -Wno-constant-logical-operand")
string(APPEND CMAKE_CXX_FLAGS " -Wno-missing-declarations")
string(APPEND CMAKE_CXX_FLAGS " -Wno-sometimes-uninitialized")

# We have public Cython APIs which return C++ types, which are in an extern
# "C" blog (no symbol mangling) and clang doesn't like this
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-return-type-c-linkage")
string(APPEND CMAKE_CXX_FLAGS " -Wno-return-type-c-linkage")
endif()
endif()

Expand Down
4 changes: 3 additions & 1 deletion python/pyarrow/_flight.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,9 @@ cdef class FlightClient(_Weakrefable):

def close(self):
"""Close the client and disconnect."""
check_flight_status(self.client.get().Close())
client = self.client.get()
if client != NULL:
check_flight_status(client.Close())

def __del__(self):
# Not ideal, but close() wasn't originally present so
Expand Down
15 changes: 10 additions & 5 deletions python/pyarrow/includes/libarrow_flight.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,16 @@ cdef extern from "arrow/flight/api.h" namespace "arrow" nogil:
c_bool Equals(const CLocation& other)

@staticmethod
CResult[CLocation] Parse(c_string& uri_string)
CResult[CLocation] Parse(const c_string& uri_string)

@staticmethod
CResult[CLocation] ForGrpcTcp(c_string& host, int port)
CResult[CLocation] ForGrpcTcp(const c_string& host, int port)

@staticmethod
CResult[CLocation] ForGrpcTls(c_string& host, int port)
CResult[CLocation] ForGrpcTls(const c_string& host, int port)

@staticmethod
CResult[CLocation] ForGrpcUnix(c_string& path)
CResult[CLocation] ForGrpcUnix(const c_string& path)

cdef cppclass CFlightEndpoint" arrow::flight::FlightEndpoint":
CFlightEndpoint()
Expand Down Expand Up @@ -172,7 +172,9 @@ cdef extern from "arrow/flight/api.h" namespace "arrow" nogil:
CResult[unique_ptr[CFlightInfo]] Next()

cdef cppclass CSimpleFlightListing" arrow::flight::SimpleFlightListing":
CSimpleFlightListing(vector[CFlightInfo]&& info)
# This doesn't work with Cython >= 3
# CSimpleFlightListing(vector[CFlightInfo]&& info)
CSimpleFlightListing(const vector[CFlightInfo]& info)

cdef cppclass CFlightPayload" arrow::flight::FlightPayload":
shared_ptr[CBuffer] descriptor
Expand Down Expand Up @@ -308,7 +310,10 @@ cdef extern from "arrow/flight/api.h" namespace "arrow" nogil:
cdef cppclass CCallHeaders" arrow::flight::CallHeaders":
cppclass const_iterator:
pair[c_string, c_string] operator*()
# For Cython < 3
const_iterator operator++()
# For Cython >= 3
const_iterator operator++(int)
bint operator==(const_iterator)
bint operator!=(const_iterator)
const_iterator cbegin()
Expand Down
7 changes: 6 additions & 1 deletion python/pyarrow/ipc.pxi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,11 @@ cdef class MessageReader(_Weakrefable):

def __iter__(self):
while True:
yield self.read_next_message()
try:
yield self.read_next_message()
except StopIteration:
# For Cython >= 3.0.0
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it is this code that gives problems to be written in a way that works for both cython 0.29 and 3, but a potential different way of writing it:

def __iter__(self):
    return self

def __next__(self):
    return self.read_next_message()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'll try it.


def read_next_message(self):
"""
Expand Down Expand Up @@ -660,6 +664,7 @@ cdef class RecordBatchReader(_Weakrefable):
try:
yield self.read_next_batch()
except StopIteration:
# For Cython >= 3.0.0
return

@property
Expand Down
2 changes: 1 addition & 1 deletion python/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

[build-system]
requires = [
"cython >= 0.29.31,<3",
"cython >= 0.29.31",
"oldest-supported-numpy>=0.14",
"setuptools_scm",
"setuptools >= 40.1.0",
Expand Down
2 changes: 1 addition & 1 deletion python/requirements-build.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cython>=0.29.31,<3
cython>=0.29.31
oldest-supported-numpy>=0.14
setuptools_scm
setuptools>=38.6.0
2 changes: 1 addition & 1 deletion python/requirements-wheel-build.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cython>=0.29.31,<3
cython>=0.29.31
oldest-supported-numpy>=0.14
setuptools_scm
setuptools>=58
Expand Down
6 changes: 3 additions & 3 deletions python/setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@
# Check if we're running 64-bit Python
is_64_bit = sys.maxsize > 2**32

if Cython.__version__ < '0.29.31' or Cython.__version__ >= '3.0':
if Cython.__version__ < '0.29.31':
raise Exception(
'Please update your Cython version. Supported Cython >= 0.29.31, < 3.0')
'Please update your Cython version. Supported Cython >= 0.29.31')

setup_dir = os.path.abspath(os.path.dirname(__file__))

Expand Down Expand Up @@ -492,7 +492,7 @@ def has_ext_modules(foo):
'pyarrow/_generated_version.py'),
'version_scheme': guess_next_dev_version
},
setup_requires=['setuptools_scm', 'cython >= 0.29.31,<3'] + setup_requires,
setup_requires=['setuptools_scm', 'cython >= 0.29.31'] + setup_requires,
install_requires=install_requires,
tests_require=['pytest', 'pandas', 'hypothesis'],
python_requires='>=3.8',
Expand Down