-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GH-36730: [Python] Add support for Cython 3.0.0 #37097
Merged
Merged
Changes from all commits
Commits
Show all changes
41 commits
Select commit
Hold shift + click to select a range
93c61c7
GH-36730: [Python] Add support for Cython 3.0.0
kou dd9f5e5
Fix bad merge conflict resolution
danepitkin 2da3f31
Revert unnecessary edits of compute.rst
danepitkin 01025e3
Try class instead of classmethod in __reduce__
danepitkin cf1ea3f
Revert "Try class instead of classmethod in __reduce__"
danepitkin e2d631e
Fix test_fragments_repr
danepitkin d1e7d8a
Update substrait test
danepitkin f2937d8
Try noexcept on substrait function
danepitkin 25ba028
Move the noexcept to the correct spot
danepitkin 097f6f3
Fix cloudpickle test
danepitkin 3263eaf
Also run test_fs.py with cloudpickle
danepitkin 5241c24
Lint
danepitkin 021e133
Delete accidental file
danepitkin ee9f049
Make _reconstruct staticmethods
danepitkin 0cd3439
Add test for MapScalar.__iter__
danepitkin 148d285
Parametrize all pickling tests to use both the pickle and cloudpickle…
danepitkin 1d99984
Lint
danepitkin fe24f0b
Remove unnecessary test function
danepitkin ac2d11f
Add Cython<3 dev CI job
danepitkin 32a4402
Fix docker_compose.yml
danepitkin 3b10534
Update CI config
danepitkin bdec809
Fix dockerfile
danepitkin 2df074f
Apply suggestions from code review
danepitkin 91257db
Handle repr non-determinism in test case
danepitkin de35878
Ignore numpydocs warnings
danepitkin a9d99d4
Try fixing numpydoc warning ignores
danepitkin 2d59f16
Try adding docstrings to cpdef enums
danepitkin e5e3cea
Ignore EnumType parameter docstrings checks during numpydoc validation
danepitkin f47f806
Remove print statement
danepitkin 676b538
Disable debug builds for cuda and ubuntu 20 on azure
danepitkin d89fde1
Use backwards-compatible Enum base class
danepitkin 59fffdc
Update x-hierarchy in docker-compose
danepitkin 7c56dcc
Disable gdb tests for non-debug builds
danepitkin 27d1128
Revert "Disable gdb tests for non-debug builds"
danepitkin 23e8a3e
Revert "Update x-hierarchy in docker-compose"
danepitkin 4ec5188
Revert "Disable debug builds for cuda and ubuntu 20 on azure"
danepitkin 90f5321
Disable Cython 3
danepitkin 6f32fcf
Clean up quotes in dockerfile
danepitkin ec5754c
Add todo, revert back to classmethod
danepitkin eee7573
Revert formatting change
danepitkin 3c4f581
Revert typo
danepitkin File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
# Licensed to the Apache Software Foundation (ASF) under one | ||
# or more contributor license agreements. See the NOTICE file | ||
# distributed with this work for additional information | ||
# regarding copyright ownership. The ASF licenses this file | ||
# to you under the Apache License, Version 2.0 (the | ||
# "License"); you may not use this file except in compliance | ||
# with the License. You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, | ||
# software distributed under the License is distributed on an | ||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
# KIND, either express or implied. See the License for the | ||
# specific language governing permissions and limitations | ||
# under the License. | ||
|
||
ARG repo | ||
ARG arch | ||
ARG python=3.8 | ||
FROM ${repo}:${arch}-conda-python-${python} | ||
|
||
RUN mamba install -q -y "cython<3" && \ | ||
mamba clean --all |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this function cannot raise a Python exception? Its code is definitely non-trivial, so the "solution" here looks more like a bandaid.
In the interest of moving this forward, can you open a bug for this and we'll revisit later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function executes user-defined python code, which absolutely can raise an exception. In fact, the test cases specifically do raise an exception. The problem is that the C++ functionality explicitly chooses to ignore it and return its own error. The pyarrow binding is designed with this in mind, so adding
noexcept
is mimicking the expected behavior of previous cython versions. I'll file an issue to request refactoring of this feature.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #37235
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll see if I can fix this in a separate PR first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would certainly be nice to fix that, but FWIW I don't think that should hold up this PR even longer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI I haven't attempted #37235. I hope its okay to merge as-is given how long this PR has taken.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you just add a comment pointing to GH-37235?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do!