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

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

Closed
kou 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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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
2 changes: 1 addition & 1 deletion docs/source/cpp/compute.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down
29 changes: 18 additions & 11 deletions python/CMakeLists.txt
View file
Open in desktop
Original file line numberDiff line numberDiff 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
2 changes: 1 addition & 1 deletion python/pyarrow/_dataset_parquet.pyx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
thrift_string_size_limit=self.thrift_string_size_limit,
thrift_container_size_limit=self.thrift_container_size_limit,
)
return type(self)._reconstruct, (kwargs,)
return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down
10 changes: 7 additions & 3 deletions python/pyarrow/_flight.pyx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
cdef shared_ptr[CMetadataRecordBatchReader] reader

def __iter__(self):
while True:
yield self.read_chunk()
return self

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

@property
def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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
15 changes: 8 additions & 7 deletions python/pyarrow/ipc.pxi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
return result

def __iter__(self):
while True:
yield self.read_next_message()
return self

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

def read_next_message(self):
"""
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
# cdef block is in lib.pxd

def __iter__(self):
while True:
try:
yield self.read_next_batch()
except StopIteration:
return
return self

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

@property
def schema(self):
Expand Down
4 changes: 2 additions & 2 deletions python/pyarrow/scalar.pxi
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
Iterate over this element's values.
"""
arr = self.values
if array is None:
raise StopIteration
if arr is None:
return
for k, v in zip(arr.field('key'), arr.field('value')):
yield (k.as_py(), v.as_py())

@pitroupitrouJul 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?

You can't use yield in a __next__ function.
Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.

@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)

@koukouJul 20, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.

I reverted the __iter__ + __next__ change.


Expand Down
2 changes: 1 addition & 1 deletion python/pyproject.toml
View file
Open in desktop
Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

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

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

 Closed

kou
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 all 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
docs/source/cpp/compute.rst

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -1621,7 +1621,7 @@ do not detect overflow. They are alsoavailable in an overflow-checking variant,
suffixed ``_checked``, which returns an ``Invalid`` :class:`Status` when 
overflow is detected.

+------------------------+-------+-------------+-------------+--------------------------------+-------+
+-------------------------+-------+-------------+-------------+--------------------------------+-------+
| Function name | Arity | Input types | Output type | Options class | Notes |
+=========================+=======+=============+=============+================================+=======+
| cumulative_sum | Unary | Numeric | Numeric | :struct:`CumulativeOptions` | \(1) |
Expand Down

29 changes: 18 additions & 11 deletions
python/CMakeLists.txt

View file


Open in desktop

Original file line numberDiff line numberDiff 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

2 changes: 1 addition & 1 deletion
python/pyarrow/_dataset_parquet.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -741,7 +741,7 @@ cdef class ParquetFragmentScanOptions(FragmentScanOptions):
 thrift_string_size_limit=self.thrift_string_size_limit,
 thrift_container_size_limit=self.thrift_container_size_limit,
 )
 return type(self)._reconstruct, (kwargs,)
 return ParquetFragmentScanOptions._reconstruct, (kwargs,)


cdef class ParquetFactoryOptions(_Weakrefable):
Expand Down

10 changes: 7 additions & 3 deletions
python/pyarrow/_flight.pyx

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -982,8 +982,10 @@ cdef class _MetadataRecordBatchReader(_Weakrefable, _ReadPandasMixin):
 cdef shared_ptr[CMetadataRecordBatchReader] reader

 def __iter__(self):
 while True:
 yield self.read_chunk()
 return self

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

 @property
 def schema(self):
Expand DownExpand Up@@ -1625,7 +1627,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 numberDiff line numberDiff 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 DownExpand 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 DownExpand 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

15 changes: 8 additions & 7 deletions
python/pyarrow/ipc.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -436,8 +436,10 @@ cdef class MessageReader(_Weakrefable):
 return result

 def __iter__(self):
 while True:
 yield self.read_next_message()
 return self

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

 def read_next_message(self):
 """
Expand DownExpand Up@@ -656,11 +658,10 @@ cdef class RecordBatchReader(_Weakrefable):
 # cdef block is in lib.pxd

 def __iter__(self):
 while True:
 try:
 yield self.read_next_batch()
 except StopIteration:
 return
 return self

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

 @property
 def schema(self):
Expand Down

4 changes: 2 additions & 2 deletions
python/pyarrow/scalar.pxi

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -792,8 +792,8 @@ cdef class MapScalar(ListScalar):
 Iterate over this element's values.
 """
 arr = self.values
 if array is None:
raise StopIteration
 if arr is None:
return
 for k, v in zip(arr.field('key'), arr.field('value')):
 yield (k.as_py(), v.as_py())
@pitroupitrouJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Do you actually test these changes before pushing them?
You can't use yield in a __next__ function.

Either you write __iter__ as a generator (using yield), or you write a pair of __iter__ and __next__ functions (without yield). You can't mix the two styles.
@jorisvandenbosschejorisvandenbosscheJul 20, 2023

Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

And FWIW, for this case, I would leave the __iter__ as is (in the other cases such as RecordBatchReader, we already have a public "next" method that can just be called from __next__)
@koukouJul 20, 2023

Copy link


Copy Markdown

MemberAuthor
Choose a reason for hiding this comment

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

Sorry. I didn't test on local before I pushed a commit because I don't have enough time yesterday.
I reverted the __iter__ + __next__ change.

Expand Down

2 changes: 1 addition & 1 deletion
python/pyproject.toml

View file


Open in desktop

Original file line numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 numberDiff line numberDiff 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 DownExpand 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