Skip to content

Define pandas and polars compatibility testing versions in dependencies.yaml - #22189

Merged
rapids-bot[bot] merged 11 commits into
NVIDIA:mainfrom
Matt711:imp/ci/pandas-polars-compat
Jun 2, 2026
Merged

Define pandas and polars compatibility testing versions in dependencies.yaml#22189
rapids-bot[bot] merged 11 commits into
NVIDIA:mainfrom
Matt711:imp/ci/pandas-polars-compat

Conversation

@Matt711

@Matt711 Matt711 commented Apr 17, 2026

Copy link
Copy Markdown
Member

Description

Replaces the pip index versions PyPI network call in ci/cudf_pandas_scripts/run_tests.sh and ci/test_wheel_cudf_polars.sh with explicit version lists defined in dependencies.yaml. Adds a new ci/utils/get_matrix_values.py utility to read those lists. For each version, a per-version requirements.txt is generated via rapids-dependency-file-generator and separate virtual environment is created with all dependency including downloaded wheels, avoiding environment contamination between runs.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@Matt711 Matt711 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 17, 2026
@copy-pr-bot

copy-pr-bot Bot commented Apr 17, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@Matt711

Matt711 commented Apr 17, 2026

Copy link
Copy Markdown
Member Author

/ok to test 81b0f70

Comment thread dependencies.yaml Outdated
Comment on lines +1410 to +1413
- matrix:
polars_compat_version: "1.34"
packages:
- polars==1.34.*

@Matt711 Matt711 Apr 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why is the polars binary missing for version 1.34?

RAPIDS logger » [04/17/26 13:49:13]
┌───────────────────────────────────────┐
|    Running tests for polars 1.34.*    |
└───────────────────────────────────────┘

ImportError while loading conftest '/__w/cudf/cudf/python/cudf_polars/tests/conftest.py'.
tests/conftest.py:9: in <module>
    import cudf_polars.callback
cudf_polars/__init__.py:14: in <module>
    from cudf_polars.callback import execute_with_cudf
cudf_polars/callback.py:20: in <module>
    from polars.exceptions import ComputeError, PerformanceWarning
../../venv_polars_1.34/lib/python3.14/site-packages/polars/__init__.py:56: in <module>
    from polars import api, exceptions, plugins, selectors
../../venv_polars_1.34/lib/python3.14/site-packages/polars/api.py:6: in <module>
    import polars._reexport as pl
../../venv_polars_1.34/lib/python3.14/site-packages/polars/_reexport.py:3: in <module>
    from polars.dataframe import DataFrame
../../venv_polars_1.34/lib/python3.14/site-packages/polars/dataframe/__init__.py:1: in <module>
    from polars.dataframe.frame import DataFrame
../../venv_polars_1.34/lib/python3.14/site-packages/polars/dataframe/frame.py:32: in <module>
    from polars import functions as F
../../venv_polars_1.34/lib/python3.14/site-packages/polars/functions/__init__.py:32: in <module>
    from polars.functions.lazy import (
../../venv_polars_1.34/lib/python3.14/site-packages/polars/functions/lazy.py:8: in <module>
    import polars.selectors as cs
../../venv_polars_1.34/lib/python3.14/site-packages/polars/selectors.py:33: in <module>
    from polars.expr import Expr
../../venv_polars_1.34/lib/python3.14/site-packages/polars/expr/__init__.py:1: in <module>
    from polars.expr.expr import Expr
../../venv_polars_1.34/lib/python3.14/site-packages/polars/expr/expr.py:66: in <module>
    from polars.meta import thread_pool_size
../../venv_polars_1.34/lib/python3.14/site-packages/polars/meta/__init__.py:3: in <module>
    from polars.meta.build import build_info
../../venv_polars_1.34/lib/python3.14/site-packages/polars/meta/build.py:5: in <module>
    from polars._utils.polars_version import get_polars_version
../../venv_polars_1.34/lib/python3.14/site-packages/polars/_utils/polars_version.py:9: in <module>
    warnings.warn("Polars binary is missing!", stacklevel=2)
E   UserWarning: Polars binary is missing!

RAPIDS logger » [04/17/26 13:49:13]
┌──────────────────────────────────────┐
|    Tests failed for polars 1.34.*    |
└──────────────────────────────────────┘

@Matt711

Matt711 commented Apr 17, 2026

Copy link
Copy Markdown
Member Author

/ok to test ffd9949

@Matt711
Matt711 force-pushed the imp/ci/pandas-polars-compat branch from ffd9949 to 8443e4b Compare April 17, 2026 14:43
@Matt711

Matt711 commented Apr 17, 2026

Copy link
Copy Markdown
Member Author

/ok to test fb4de7a

@wence- wence- mentioned this pull request Apr 23, 2026
3 tasks
@wence-

wence- commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

I merged this into #22265 and will continue there.

rapids-bot Bot pushed a commit that referenced this pull request Apr 23, 2026
This is a simpler fix for the problem #22189 is aiming to solve that should get us unblocked in the near term.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: #22273
shrshi pushed a commit to shrshi/cudf that referenced this pull request May 12, 2026
This is a simpler fix for the problem NVIDIA#22189 is aiming to solve that should get us unblocked in the near term.

Authors:
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Bradley Dice (https://github.com/bdice)

URL: NVIDIA#22273
@mroeschke

Copy link
Copy Markdown
Contributor

/ok to test f40253e

@mroeschke

Copy link
Copy Markdown
Contributor

/ok to test 5f7106e

@mroeschke

Copy link
Copy Markdown
Contributor

/ok to test dc2d077

@mroeschke
mroeschke marked this pull request as ready for review May 28, 2026 23:01
@mroeschke
mroeschke requested review from a team as code owners May 28, 2026 23:01
@mroeschke
mroeschke requested a review from jameslamb May 28, 2026 23:01
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 675a350c-4b05-4a43-8426-20f15d51e070

📥 Commits

Reviewing files that changed from the base of the PR and between dc2d077 and 6565365.

📒 Files selected for processing (2)
  • ci/cudf_pandas_scripts/run_tests.sh
  • dependencies.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
  • dependencies.yaml
  • ci/cudf_pandas_scripts/run_tests.sh

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Refactored pandas and polars compatibility testing infrastructure for improved reliability and maintainability
    • Enhanced per-version test isolation and dependency management for compatibility testing

Walkthrough

Adds dependency matrix entries for pandas and polars, a CLI to extract matrix values from dependencies.yaml, and refactors CI scripts to run per-compatibility-version tests in isolated virtual environments using generated per-version requirement files.

Changes

Compatibility version test infrastructure

Layer / File(s) Summary
Compatibility testing targets and dependency matrices in dependencies.yaml
dependencies.yaml
Adds files.test_cudf_pandas_compat and files.test_cudf_polars_compat targets and dependencies.test_cudf_pandas_compat, dependencies.test_cudf_polars_compat groups that pin pandas/polars versions and apply NumPy/CuPy CUDA-specific bounds.
Matrix value extraction utility
ci/utils/get_matrix_values.py
Adds CLI get_matrix_values to load dependencies.yaml, validate presence of files[file_key].matrix[matrix_var], and print stringified matrix values.
Pandas compatibility per-version venv loop
ci/cudf_pandas_scripts/run_tests.sh
Derives pandas compat versions from the matrix, generates per-version requirements, creates and activates per-version venvs, installs test deps (wheelhouse + generated requirements under existing constraints), runs pytest, then deactivates and removes venv and requirements.
Polars compatibility per-version venv loop and tracking
ci/test_wheel_cudf_polars.sh
Extracts polars compat versions from the matrix, generates per-version requirements, creates/activates per-version venvs, installs cudf_polars wheels and generated polars requirements, runs tests, captures exit codes, appends versions to PASSED/FAILED lists, and cleans up venvs and files.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes


Possibly related PRs

  • rapidsai/cudf#22717: Changes CuPy exclusion pins in dependencies.yaml that affect generated per-version requirement installs.

Suggested labels

Python, cudf.pandas, cudf-polars


Suggested reviewers

  • mroeschke
  • wence-
  • bdice
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title directly describes the main change: defining pandas and polars compatibility testing versions in dependencies.yaml.
Description check ✅ Passed The PR description clearly explains the purpose: replacing pip index version calls with explicit lists in dependencies.yaml and introducing isolated venv testing.
Linked Issues check ✅ Passed The PR fully implements the requirements from issue #21968: defines pandas/polars versions in dependencies.yaml, creates get_matrix_values.py utility, generates per-version requirements files, and creates isolated virtual environments for testing.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the linked issue: refactored test scripts, added utility function, and defined compatibility matrices in dependencies.yaml with no unrelated modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
ci/test_wheel_cudf_polars.sh (1)

36-83: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add a GPU availability preflight check before the test loop.

The script runs GPU-dependent tests without an explicit GPU check. Add a fail-fast probe so CI errors are immediate and actionable.

Suggested patch
 PASSED=()
 FAILED=()
 
+if ! nvidia-smi -L >/dev/null 2>&1; then
+    rapids-logger "GPU not detected; cannot run cudf_polars compatibility tests"
+    exit 1
+fi
+
 for version in "${VERSIONS[@]}"; do
As per coding guidelines, "Verify GPU availability checks before tests".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_wheel_cudf_polars.sh` around lines 36 - 83, Add a GPU availability
preflight immediately before the for version in "${VERSIONS[@]}" loop: run a
probe (e.g., call nvidia-smi or a small Python/cuda import) and if it fails log
an error with rapids-logger and exit non‑zero so CI fails fast; ensure the check
references the same environment used by tests (GPU presence for RAPIDS/CUDA) and
guard the loop by returning non-zero when no GPUs are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/cudf_pandas_scripts/run_tests.sh`:
- Around line 90-125: The loop that sets up env_name, activates the venv and
installs deps assumes a GPU is present but never verifies it; add a fail-fast
GPU preflight check (e.g., run `nvidia-smi -L` and exit non-zero with a clear
error message) inside the loop after sourcing "${env_name}/bin/activate" and
before the `python -m pytest -p cudf.pandas` invocation so tests abort early on
CI when no GPU is available.

In `@ci/test_wheel_cudf_polars.sh`:
- Around line 48-69: The venv creation/activation and dependency installation
steps (env_name, python -m venv --clear, source "${env_name}/bin/activate", and
rapids-pip-retry install) are currently unguarded so failures are ignored;
modify the script to immediately stop and report an error if any of these steps
fail—either enable errexit (set -e) around the venv/install block or explicitly
check the exit status after python -m venv, after source
"${env_name}/bin/activate", and after rapids-pip-retry install and call a
failure handler (logging via rapids-logger and exit nonzero) so tests never run
in a broken environment; apply the same guarding pattern to the later venv block
referenced for lines 91-103.

In `@ci/utils/get_matrix_values.py`:
- Around line 24-30: The code currently treats the fetched `values` as iterable,
which will iterate characters if a scalar string is returned; update the block
that reads `values = matrix.get(matrix_var)` to validate the type before
iterating: check that `values` is a sequence (e.g., list/tuple or a
collections.abc.Sequence) and explicitly reject strings/bytes, and if it is not
a proper sequence raise a RuntimeError (include `matrix_var` and `file_key` in
the message) instead of silently iterating; then safely return `[str(v) for v in
values]`.

---

Outside diff comments:
In `@ci/test_wheel_cudf_polars.sh`:
- Around line 36-83: Add a GPU availability preflight immediately before the for
version in "${VERSIONS[@]}" loop: run a probe (e.g., call nvidia-smi or a small
Python/cuda import) and if it fails log an error with rapids-logger and exit
non‑zero so CI fails fast; ensure the check references the same environment used
by tests (GPU presence for RAPIDS/CUDA) and guard the loop by returning non-zero
when no GPUs are detected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f0b4f98b-c72e-4ebd-ba3f-42c22262e657

📥 Commits

Reviewing files that changed from the base of the PR and between 7a9a1db and dc2d077.

📒 Files selected for processing (4)
  • ci/cudf_pandas_scripts/run_tests.sh
  • ci/test_wheel_cudf_polars.sh
  • ci/utils/get_matrix_values.py
  • dependencies.yaml

Comment on lines 90 to 125
if version_lte "${RAPIDS_PY_VERSION}" "3.13"; then
for version in "${versions[@]}"; do
echo "Installing pandas version: ${version}"
# This loop tests cudf.pandas compatibility with older pandas-numpy versions,
# requiring numpy<2. cupy>=14 dropped support for numpy<2, so we explicitly
# downgrade cupy here to avoid an import failure when cupy tries
# to load against the older numpy.
set +e
# We're iterating over a range of versions that might not all have
# supported wheels, so our best bet for now is to disallow source
# distributions.
rapids-pip-retry install "numpy>=1.26,<2.0a0" "pandas==${version}" "cupy-cuda${RAPIDS_CUDA_VERSION%%.*}x<14" --only-binary=:all:
INSTALL_SUCCESS=$?
set -e
if [[ ${INSTALL_SUCCESS} -ne 0 ]]; then
echo "Failed to install pandas ${version} with numpy<2 for Python ${RAPIDS_PY_VERSION}. Skipping tests for this version."
continue
fi
rapids-logger "Testing cudf.pandas compatibility with pandas ${version}.*"

# Generate requirements for this pandas compat version.
# Each entry pins numpy<2 + the specific pandas minor line + the CUDA-appropriate cupy<14.
# cupy>=14 dropped support for numpy<2 (see https://github.com/cupy/cupy/issues/9709).
pandas_requirements_txt="pandas-compat-${version}-requirements.txt"
rapids-dependency-file-generator \
--config dependencies.yaml \
--file-key test_cudf_pandas_compat \
--output requirements \
--matrix "cuda=${RAPIDS_CUDA_VERSION};pandas_compat_version=${version}" \
> "${pandas_requirements_txt}"

env_name="venv_pandas_${version}"
python -m venv --clear "${env_name}"
# shellcheck disable=SC1090
source "${env_name}/bin/activate"

# notes:
#
# * echo to expand wildcard before adding `[test,cudf-pandas-tests]` requires for pip
# * need to provide --constraint="${PIP_CONSTRAINT}" because that environment variable is
# ignored if any other --constraint are passed via the CLI
#
rapids-pip-retry install \
-v \
--constraint ./constraints.txt \
--constraint "${PIP_CONSTRAINT}" \
"$(echo "${CUDF_WHEELHOUSE}"/cudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)[test,cudf-pandas-tests]" \
"$(echo "${LIBCUDF_WHEELHOUSE}"/libcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
"$(echo "${PYLIBCUDF_WHEELHOUSE}"/pylibcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
-r "${pandas_requirements_txt}"

python -m pytest -p cudf.pandas \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add an explicit GPU preflight check before running compat tests.

This loop assumes CUDA/GPU availability but never verifies it before invoking pytest. Add a fail-fast guard (e.g., nvidia-smi -L) to produce a clear CI error.

Suggested patch
 if version_lte "${RAPIDS_PY_VERSION}" "3.13"; then
+    if ! nvidia-smi -L >/dev/null 2>&1; then
+        rapids-logger "GPU not detected; cannot run cudf.pandas compatibility tests"
+        exit 1
+    fi
     for version in "${versions[@]}"; do
As per coding guidelines, "Verify GPU availability checks before tests".
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if version_lte "${RAPIDS_PY_VERSION}" "3.13"; then
for version in "${versions[@]}"; do
echo "Installing pandas version: ${version}"
# This loop tests cudf.pandas compatibility with older pandas-numpy versions,
# requiring numpy<2. cupy>=14 dropped support for numpy<2, so we explicitly
# downgrade cupy here to avoid an import failure when cupy tries
# to load against the older numpy.
set +e
# We're iterating over a range of versions that might not all have
# supported wheels, so our best bet for now is to disallow source
# distributions.
rapids-pip-retry install "numpy>=1.26,<2.0a0" "pandas==${version}" "cupy-cuda${RAPIDS_CUDA_VERSION%%.*}x<14" --only-binary=:all:
INSTALL_SUCCESS=$?
set -e
if [[ ${INSTALL_SUCCESS} -ne 0 ]]; then
echo "Failed to install pandas ${version} with numpy<2 for Python ${RAPIDS_PY_VERSION}. Skipping tests for this version."
continue
fi
rapids-logger "Testing cudf.pandas compatibility with pandas ${version}.*"
# Generate requirements for this pandas compat version.
# Each entry pins numpy<2 + the specific pandas minor line + the CUDA-appropriate cupy<14.
# cupy>=14 dropped support for numpy<2 (see https://github.com/cupy/cupy/issues/9709).
pandas_requirements_txt="pandas-compat-${version}-requirements.txt"
rapids-dependency-file-generator \
--config dependencies.yaml \
--file-key test_cudf_pandas_compat \
--output requirements \
--matrix "cuda=${RAPIDS_CUDA_VERSION};pandas_compat_version=${version}" \
> "${pandas_requirements_txt}"
env_name="venv_pandas_${version}"
python -m venv --clear "${env_name}"
# shellcheck disable=SC1090
source "${env_name}/bin/activate"
# notes:
#
# * echo to expand wildcard before adding `[test,cudf-pandas-tests]` requires for pip
# * need to provide --constraint="${PIP_CONSTRAINT}" because that environment variable is
# ignored if any other --constraint are passed via the CLI
#
rapids-pip-retry install \
-v \
--constraint ./constraints.txt \
--constraint "${PIP_CONSTRAINT}" \
"$(echo "${CUDF_WHEELHOUSE}"/cudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)[test,cudf-pandas-tests]" \
"$(echo "${LIBCUDF_WHEELHOUSE}"/libcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
"$(echo "${PYLIBCUDF_WHEELHOUSE}"/pylibcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
-r "${pandas_requirements_txt}"
python -m pytest -p cudf.pandas \
if version_lte "${RAPIDS_PY_VERSION}" "3.13"; then
if ! nvidia-smi -L >/dev/null 2>&1; then
rapids-logger "GPU not detected; cannot run cudf.pandas compatibility tests"
exit 1
fi
for version in "${versions[@]}"; do
rapids-logger "Testing cudf.pandas compatibility with pandas ${version}.*"
# Generate requirements for this pandas compat version.
# Each entry pins numpy<2 + the specific pandas minor line + the CUDA-appropriate cupy<14.
# cupy>=14 dropped support for numpy<2 (see https://github.com/cupy/cupy/issues/9709).
pandas_requirements_txt="pandas-compat-${version}-requirements.txt"
rapids-dependency-file-generator \
--config dependencies.yaml \
--file-key test_cudf_pandas_compat \
--output requirements \
--matrix "cuda=${RAPIDS_CUDA_VERSION};pandas_compat_version=${version}" \
> "${pandas_requirements_txt}"
env_name="venv_pandas_${version}"
python -m venv --clear "${env_name}"
# shellcheck disable=SC1090
source "${env_name}/bin/activate"
# notes:
#
# * echo to expand wildcard before adding `[test,cudf-pandas-tests]` requires for pip
# * need to provide --constraint="${PIP_CONSTRAINT}" because that environment variable is
# ignored if any other --constraint are passed via the CLI
#
rapids-pip-retry install \
-v \
--constraint ./constraints.txt \
--constraint "${PIP_CONSTRAINT}" \
"$(echo "${CUDF_WHEELHOUSE}"/cudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)[test,cudf-pandas-tests]" \
"$(echo "${LIBCUDF_WHEELHOUSE}"/libcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
"$(echo "${PYLIBCUDF_WHEELHOUSE}"/pylibcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
-r "${pandas_requirements_txt}"
python -m pytest -p cudf.pandas \
🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 108-108: Not following: ./bin/activate was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/cudf_pandas_scripts/run_tests.sh` around lines 90 - 125, The loop that
sets up env_name, activates the venv and installs deps assumes a GPU is present
but never verifies it; add a fail-fast GPU preflight check (e.g., run
`nvidia-smi -L` and exit non-zero with a clear error message) inside the loop
after sourcing "${env_name}/bin/activate" and before the `python -m pytest -p
cudf.pandas` invocation so tests abort early on CI when no GPU is available.

Comment on lines +48 to +69
env_name="venv_polars_${version}"
python -m venv --clear "${env_name}"
# shellcheck disable=SC1090
source "${env_name}/bin/activate"

rapids-logger "Installing cudf_polars and its dependencies for polars ${version}.*"

# notes:
#
# * echo to expand wildcard before adding `[test]` requires for pip
# * just providing --constraint="${PIP_CONSTRAINT}" to be explicit, and because
# that environment variable is ignored if any other --constraint are passed via the CLI
#
rapids-pip-retry install \
-v \
--prefer-binary \
--constraint "${PIP_CONSTRAINT}" \
"$(echo "${CUDF_POLARS_WHEELHOUSE}"/cudf_polars_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)[test,dask,ray]" \
"$(echo "${LIBCUDF_WHEELHOUSE}"/libcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
"$(echo "${PYLIBCUDF_WHEELHOUSE}"/pylibcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
-r "polars-compat-${version}-requirements.txt"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard venv setup/install failures to avoid running tests in a bad environment.

With set +e, failures in venv creation/activation/dependency install do not stop execution. The script can continue into test execution with a partially configured or wrong environment.

Suggested patch
-    python -m venv --clear "${env_name}"
+    if ! python -m venv --clear "${env_name}"; then
+        EXITCODE=1
+        FAILED+=("${version}")
+        rapids-logger "Failed to create venv for polars ${version}.*"
+        continue
+    fi
     # shellcheck disable=SC1090
-    source "${env_name}/bin/activate"
+    if ! source "${env_name}/bin/activate"; then
+        EXITCODE=1
+        FAILED+=("${version}")
+        rapids-logger "Failed to activate venv for polars ${version}.*"
+        rm -rf "${env_name}" "${polars_requirements_txt}"
+        continue
+    fi
...
-    rapids-pip-retry install \
+    if ! rapids-pip-retry install \
         -v \
         --prefer-binary \
         --constraint "${PIP_CONSTRAINT}" \
         "$(echo "${CUDF_POLARS_WHEELHOUSE}"/cudf_polars_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)[test,dask,ray]" \
         "$(echo "${LIBCUDF_WHEELHOUSE}"/libcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
         "$(echo "${PYLIBCUDF_WHEELHOUSE}"/pylibcudf_"${RAPIDS_PY_CUDA_SUFFIX}"*.whl)" \
-        -r "polars-compat-${version}-requirements.txt"
+        -r "polars-compat-${version}-requirements.txt"; then
+        EXITCODE=1
+        FAILED+=("${version}")
+        rapids-logger "Dependency install failed for polars ${version}.*"
+        deactivate
+        rm -rf "${env_name}" "${polars_requirements_txt}"
+        continue
+    fi

Also applies to: 91-103

🧰 Tools
🪛 Shellcheck (0.11.0)

[info] 51-51: Not following: ./bin/activate was not specified as input (see shellcheck -x).

(SC1091)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/test_wheel_cudf_polars.sh` around lines 48 - 69, The venv
creation/activation and dependency installation steps (env_name, python -m venv
--clear, source "${env_name}/bin/activate", and rapids-pip-retry install) are
currently unguarded so failures are ignored; modify the script to immediately
stop and report an error if any of these steps fail—either enable errexit (set
-e) around the venv/install block or explicitly check the exit status after
python -m venv, after source "${env_name}/bin/activate", and after
rapids-pip-retry install and call a failure handler (logging via rapids-logger
and exit nonzero) so tests never run in a broken environment; apply the same
guarding pattern to the later venv block referenced for lines 91-103.

Comment on lines +24 to +30
values = matrix.get(matrix_var)
if values is None:
raise RuntimeError(
f"Matrix variable '{matrix_var}' not found in file key '{file_key}'"
)

return [str(v) for v in values]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate matrix value type before iterating.

On Line 30, a scalar YAML value would be iterated as characters and silently produce invalid output. Fail fast unless the matrix value is a sequence.

Suggested patch
-    values = matrix.get(matrix_var)
+    values = matrix.get(matrix_var)
     if values is None:
         raise RuntimeError(
             f"Matrix variable '{matrix_var}' not found in file key '{file_key}'"
         )
+    if not isinstance(values, (list, tuple)):
+        raise RuntimeError(
+            f"Matrix variable '{matrix_var}' in file key '{file_key}' must be a list"
+        )
 
     return [str(v) for v in values]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/utils/get_matrix_values.py` around lines 24 - 30, The code currently
treats the fetched `values` as iterable, which will iterate characters if a
scalar string is returned; update the block that reads `values =
matrix.get(matrix_var)` to validate the type before iterating: check that
`values` is a sequence (e.g., list/tuple or a collections.abc.Sequence) and
explicitly reject strings/bytes, and if it is not a proper sequence raise a
RuntimeError (include `matrix_var` and `file_key` in the message) instead of
silently iterating; then safely return `[str(v) for v in values]`.

@vyasr vyasr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks!

Comment thread ci/cudf_pandas_scripts/run_tests.sh Outdated

# Generate requirements for this pandas compat version.
# Each entry pins numpy<2 + the specific pandas minor line + the CUDA-appropriate cupy<14.
# cupy>=14 dropped support for numpy<2 (see https://github.com/cupy/cupy/issues/9709).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that this issue was resolved, so we only have to exclude cupy==14.0.0, later 14.x releases are fine for the CAI issue. However, numpy 2 support was indeed dropped, which I think is the main point, so maybe we just need a different link here?

@mroeschke

Copy link
Copy Markdown
Contributor

/merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEA] Define multiple pandas/Polars version testing in dependencies.yml

4 participants