GH-36744: [Python][Packaging] Add upper pin for cython<3 to pyarrow build dependencies #36743
Conversation
github-actions
bot
added
Component: Python
awaiting committer review
jorisvandenbossche
changed the title
|
|
|
@github-actions crossbow submit example-python-minimal-build-fedora-conda example-python-minimal-build-ubuntu-venv |
This comment was marked as outdated.
This comment was marked as outdated.
|
@github-actions crossbow submit -g python |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
aren't the changes on setup.py required?
diff --git a/python/setup.py b/python/setup.py
index f06cb5a..dc52967 100755
--- a/python/setup.py
+++ b/python/setup.py
@@ -40,8 +40,9 @@ import Cython
# Check if we're running 64-bit Python
is_64_bit = sys.maxsize > 2**32
-if Cython.__version__ < '0.29.31':
- raise Exception('Please upgrade to Cython 0.29.31 or newer')
+if Cython.__version__ < '0.29.31' or Cython.__version__ >= '3.0':
+ raise Exception(
+ 'Please update your Cython version. Supported Cython >= 0.29.31, < 3.0')
setup_dir = os.path.abspath(os.path.dirname(__file__))
@@ -491,7 +492,7 @@ setup(
'pyarrow/_generated_version.py'),
'version_scheme': guess_next_dev_version
},
- setup_requires=['setuptools_scm', 'cython >= 0.29.31'] + setup_requires,
+ setup_requires=['setuptools_scm', 'cython >= 0.29.31,<3'] + setup_requires,
|
@github-actions crossbow submit -g wheel |
|
Revision: ce27211 Submitted crossbow builds: ursacomputing/crossbow @ actions-09d93813a0
|
|
The failure on |
|
@github-actions crossbow submit example-python-minimal-build-fedora-conda example-python-minimal-build-ubuntu-venv |
github-actions
bot
added
awaiting change review
|
Revision: c8860dd Submitted crossbow builds: ursacomputing/crossbow @ actions-7fd78f41a9
|
No idea what this It's not really "required", but indeed nicer to update the error message (it would also have been clearer on the conda build). On the other hand, I am not sure it is necessary to fully disallow building with cython 3, for example for local development. But assuming we remove the pin soon again on the main branch, I added it for now.
We actually shouldn't, if we would update our install commands in CI to use pip instead of calling setup.py directly. But that's for another issue ;) |
|
to fix the |
|
Thanks! Pushed (and that's another place where we should try to avoid installing cython manually, I think) |
|
@github-actions crossbow submit -g python |
|
@jorisvandenbossche @raulcd Can we merge this once CI passes? |
|
Revision: faa87fe Submitted crossbow builds: ursacomputing/crossbow @ actions-89b7c43a1b
|
github-actions
bot
added
awaiting merge
…uild dependencies (#36743) ### Rationale for this change Although we already fixed some cython 3 build issues (#34726), some new have been introduced, which we are seeing now cython 3 is released (#36730) Adding an upper pin (<3) for the release, so we have more time (the full 14.0 release cycle) to iron out issues. * Closes: #36744 Authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
…rrow build dependencies (apache#36743) ### Rationale for this change Although we already fixed some cython 3 build issues (apache#34726), some new have been introduced, which we are seeing now cython 3 is released (apache#36730) Adding an upper pin (<3) for the release, so we have more time (the full 14.0 release cycle) to iron out issues. * Closes: apache#36744 Authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
|
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit de8df23. There were 3 benchmark results indicating a performance regression:
The full Conbench report has more details. |
…rrow build dependencies (apache#36743) ### Rationale for this change Although we already fixed some cython 3 build issues (apache#34726), some new have been introduced, which we are seeing now cython 3 is released (apache#36730) Adding an upper pin (<3) for the release, so we have more time (the full 14.0 release cycle) to iron out issues. * Closes: apache#36744 Authored-by: Joris Van den Bossche <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]>
Rationale for this change
Although we already fixed some cython 3 build issues (#34726), some new have been introduced, which we are seeing now cython 3 is released (#36730)
Adding an upper pin (<3) for the release, so we have more time (the full 14.0 release cycle) to iron out issues.