Pin pyarrow < 25 in Polars tests - #23218
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Polars CI test script updates its SPDX copyright text and pins the cloned Polars development dependency ChangesPolars CI updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| # We don't need to pick up dependencies from polars-cloud, so we remove it. | ||
| sed -i '/^polars-cloud$/d' polars/py-polars/requirements-dev.txt | ||
| # Pin pyarrow to avoid FutureWarning https://github.com/pola-rs/polars/pull/28323. | ||
| sed -i 's/^pyarrow$/pyarrow<25/' polars/py-polars/requirements-dev.txt |
There was a problem hiding this comment.
This is totally fine, don't bother restarting CI for my suggestion, but want to mention something I think I've said on PRs like this here before (but maybe not to the same audience)... there's an easier way to constrain the solve, that doesn't require regex, sed, or any knowledge of how the polars repo is laid out.
rapids-init-pip sets up a (by default, empty) constraints file at ${PIP_CONSTRAINT} and that's used below. This change would be equivalent:
echo "pyarrow<25" >> "${PIP_CONSTRAINT}"There was a problem hiding this comment.
Ah gotcha, thanks for the tip!
Curious, if a pinning in requirements.txt file conflicts with a pip constraint e.g. pyarrow>=25 with a pyarrow < 25 constraint, is the constraint ignored? The docs weren't really clear on this case https://pip.pypa.io/en/stable/user_guide/#constraints-files
There was a problem hiding this comment.
The solve will fail in that case
$ echo "pip>25" > ./requirements.txt
$ echo "pip<25" > ./constraints.txt
$ pip install -r ./requirements.txt -c ./constraints.txt
ERROR: Cannot install pip>25 because these package versions have conflicting dependencies.
The conflict is caused by:
The user requested pip>25
The user requested (constraint) pip<25
To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict
ERROR: ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflictsAt least, with recent versions of pip. I only tested with what I had laying around
$ pip --version
pip 24.0 from /usr/lib/python3/dist-packages/pip (python 3.12)Another way to think about this... pip 's solve is "install all the requirements, while not violating any constraints".
That means:
- constraints on packages that never end up actually getting installed are generally harmless
- you can have many requirements and constraints referencing the same package, as long as there's at least one package available that satisfies all of them
So freely >>-ing constraints onto constraints.txt is a great way to constraint the solve, and you don't need to care about the exact state of that file or any other requirements files / requirements from wheels.
There was a problem hiding this comment.
Shouldn't it be Addressed in #23218 (comment)>> rather than <<?
There was a problem hiding this comment.
ha yes that was a typo, fixed. Thank you for catching that.
|
/merge |
Description
Polars tests use a feather API that has been deprecated in Pyarrow 25
A fix is in progress on the Polars side pola-rs/polars#28323
Checklist