-
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-43690: [Python][CI] Simplify python/requirements-wheel-test.txt file #43691
Conversation
@github-actions crossbow submit -g wheel |
|
Revision: 035304f Submitted crossbow builds: ursacomputing/crossbow @ actions-9813ca3702 |
Leaving the python 3.13 specific ones for #43539 |
pandas<1.1.0; platform_system == "Linux" and platform_machine != "aarch64" and python_version < "3.8" | ||
pandas; platform_system == "Linux" and platform_machine != "aarch64" and python_version >= "3.8" | ||
pandas; platform_system == "Linux" and platform_machine == "aarch64" | ||
pandas<1.1.0; platform_system == "Darwin" and platform_machine != "arm64" and python_version < "3.8" | ||
pandas; platform_system == "Darwin" and platform_machine != "arm64" and python_version >= "3.8" | ||
pandas; platform_system == "Darwin" and platform_machine == "arm64" | ||
pandas<1.1.0; platform_system == "Windows" and python_version < "3.8" | ||
pandas; platform_system == "Windows" and python_version >= "3.8" | ||
pandas |
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.
Essentially we were already asking for just pandas
for everything not below 3.8, and we don't support such old python version anymore.
python/requirements-wheel-test.txt
Outdated
numpy==1.21.3; platform_system == "Linux" and platform_machine == "aarch64" and python_version < "3.11" | ||
numpy==1.23.4; python_version == "3.11" | ||
numpy==1.26.0; python_version >= "3.12" | ||
numpy==1.19.5; platform_system == "Linux" and platform_machine != "aarch64" and python_version < "3.9" | ||
numpy==1.21.3; platform_system == "Linux" and platform_machine != "aarch64" and python_version >= "3.9" and python_version < "3.11" | ||
numpy==1.21.3; platform_system == "Darwin" and platform_machine == "arm64" and python_version < "3.11" | ||
numpy==1.19.5; platform_system == "Darwin" and platform_machine != "arm64" and python_version < "3.9" | ||
numpy==1.21.3; platform_system == "Darwin" and platform_machine != "arm64" and python_version >= "3.9" and python_version < "3.11" | ||
numpy==1.19.5; platform_system == "Windows" and python_version < "3.9" | ||
numpy==1.21.3; platform_system == "Windows" and python_version >= "3.9" and python_version < "3.11" | ||
numpy==1.21.3; python_version < "3.11" | ||
numpy==1.23.5; python_version == "3.11" | ||
numpy==1.26.4; python_version == "3.12" |
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.
The only difference here is that for non-arm archs, we used numpy 1.19 instead of numpy 1.21 for python < 3.11. But given this is just to test, we can simplify to test with numpy 1.21 for all architectures
@github-actions crossbow submit -g wheel |
Revision: 7507e9e Submitted crossbow builds: ursacomputing/crossbow @ actions-b57c649d6a |
numpy==1.21.3; platform_system == "Windows" and python_version >= "3.9" and python_version < "3.11" | ||
numpy~=1.21.3; python_version < "3.11" | ||
numpy~=1.23.2; python_version == "3.11" | ||
numpy~=1.26.0; python_version == "3.12" |
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.
How is ~=
different from ==
in requirements file?
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.
Also, should be include a line for Python 3.13?
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.
How is
~=
different from==
in requirements file?
I was initially using ==
as before, but after reading https://packaging.python.org/en/latest/specifications/version-specifiers/#id5 I thought this was the best option (although I would also have to look it up again, which makes it not ideal maintenance-wise, maybe I should add a comment explaining the logic).
The ~=
is called a "compatible" release, and essentially numpy~=1.26.0
translates to >= 1.26.0, == 1.26.*
. Using this instead of ==
avoids us bumping those exact pins to the latest bug-fix release in that x.y series, as we sometimes did in the past (example)
Also, should be include a line for Python 3.13?
I had removed that again because it is being added in #43539 (anyway, depending on which PR is merged first, the other will have to fix the conflicts for this file)
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.
Added a comment
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.
LGTM, thank you @jorisvandenbossche
# version. However, there is no need to make this strictly the oldest version, | ||
# so it can be broadened to have a single version specification across platforms. | ||
# (`~=x.y.z` specifies a compatible release as `>=x.y.z, == x.y.*`) | ||
numpy~=1.21.3; python_version < "3.11" |
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.
The numpy support table says Python 3.10 supports Numpy 1.23+, while Python 3.9 supports Numpy 1.21+. Is it worth breaking these apart?
https://numpy.org/neps/nep-0029-deprecation_policy.html#support-table
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.
Ehh, maybe I'm not reading that support matrix correctly..
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 think that table is a recommendation about the versions to support for other projects (that rely on Python and numpy), not necessarily the Python/numpy version that supports each other.
So if I interpret it correctly, my reading is that, for example, at Apr 05, 2024, it is recommended to support Python 3.10+ and NumPy 1.23+.
But for the oldest NumPy version to support Python 3.10, I looked at the release notes, for example https://numpy.org/doc/stable/release/1.21.3-notes.html indicates it is the first version to support 3.10
In theory we could pin to an older version of numpy for Python 3.9 (https://numpy.org/doc/stable/release/1.19.3-notes.html), but the problem is that then we need to start using a different version depending on the architecture, because at the time numpy first supported Python 3.9 (numpy 1.19.3), there were not yet arm64 wheels.
Given it is not super critical to test exactly with the oldest, but we mostly want to test with some other version than what we are building with (which is nowadays the most recent numpy version), I decided to keep the version pins simpler (as explained in the comment that I added to the txt file)
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.
That makes sense. Your changes look good as-is. Thanks for sharing your approach!
Going to merge this, so I can update #43539 |
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 2e83aa6. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them. |
Rationale for this change
The current requirements-wheel-test.txt file has quite complex and detailed version pinning, varying per architecture. I think this can be simplified because we just want to test with some older version of numpy and pandas (and the exact version isn't that important).