Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions .agents/skills/debug-cudf-pandas/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ When the pandas test suite is run with `-p cudf.pandas`, test failures indicate
- A **proxy/dispatch bug** — the wrapping/unwrapping mechanism doesn't correctly handle a type or operation
- A **missing proxy registration** — a pandas type or return value has no registered cudf equivalent
- A **to/from_pandas conversion bug** — data is corrupted or lost when converting between cudf and pandas objects
- A **test setup bug** — the testing scripts or conftest-patch introduce an issue
- A **test setup bug** — the testing scripts or the pandas testing plugin introduce an issue
- A **dependency/environment gap** — the test requires a package (e.g. xlsxwriter) that pandas CI has but our test environment lacks, causing a different code path to execute
- A **pandas bug** — rarely, the expected behavior in the pandas test itself is wrong

Expand All @@ -37,10 +37,13 @@ The following patterns are prohibited regardless of whether they make a test pas

Before starting, verify you are at the repository root. All commands in this skill assume the working directory is the cudf repository root.

**Clean up any previous test run state.** The test runner appends `conftest-patch.py` to the pandas conftest on every invocation. If `pandas-testing/pandas-tests/` already exists from a prior run, the conftest will have duplicate hook registrations and cause spurious errors. Always delete it before running:
**Setting up and refreshing the test checkout.** The test harness lives in `pandas-testing/pandas-tests/`. The runner performs first-time setup (cloning pandas, copying the test tree, rewriting imports) *only* when the relevant directories are missing; once they exist it runs against them as-is and never refreshes them. The xfail/skip markers are applied by a pytest plugin loaded fresh on every run (`-p cudf.pandas.scripts.pandas-testing-plugin`), not by appending to the pandas `conftest.py`, so repeated runs do not accumulate duplicate hook registrations.

Because the runner never refreshes an existing checkout, do not rely on its contents being current, and be aware that any edit you make under `pandas-testing/` persists into every subsequent run. Never modify the vendored pandas test files (the `tests/**.py` tree — see the "never modify the pandas test files" rule below); a stray edit there will silently follow you and can derail your investigation. The one sanctioned in-place edit is the temporary `xfail_strict` flip in `pandas-tests/pyproject.toml` described in Step 0 — revert it (or delete the checkout) once you are done so it does not linger. Delete the checkout whenever it may be stale. In particular, if you change `python/cudf/cudf/pandas/scripts/run-pandas-tests.sh` in a way that affects what it places in `pandas-testing/`, the existing checkout will not reflect that change — delete it before re-running so the runner rebuilds it. The runner uses two independent guards: it clones pandas into `pandas-testing/pandas/` only if that directory is missing, and copies the test tree into `pandas-testing/pandas-tests/` only if *that* is missing. So removing just `pandas-tests/` re-copies a clean test tree **from the existing clone**, while picking up a new pandas version (the clone is pinned to the tag matching the installed pandas) requires removing the whole `pandas-testing/` so the clone is refetched:

```bash
rm -rf pandas-testing/pandas-tests/
rm -rf pandas-testing/pandas-tests/ # re-copy a clean test tree from the existing clone
rm -rf pandas-testing/ # full reset, e.g. after a pandas version change or a change to run-pandas-tests.sh
```

The cudf Python package is almost entirely pure Python. For **inplace installs** (e.g. `pip install -e .`), changes to `.py` files take effect immediately — no rebuild is needed. For non-inplace installs (e.g. `./build.sh`), you must either reinstall or copy changed files to site-packages.
Expand All @@ -60,27 +63,27 @@ Node IDs with parameters like `[Float64-False-False-first]` target a specific pa

---

## Step 0 — Update conftest-patch.py
## Step 0 — Update pandas-testing-plugin.py

The file `python/cudf/cudf/pandas/scripts/conftest-patch.py` contains three dictionaries that gate how tests are handled:
The file `python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py` contains three dictionaries that gate how tests are handled:

- **`NODEIDS_THAT_FAIL`** — tests marked `xfail` (expected to fail). Keys are alphabetically sorted.
- **`NODEIDS_TO_SKIP`** — tests marked `skip` (not run at all). Keys are alphabetically sorted.
- **`NODEIDS_PATHS_TO_SKIP`** — prefix-based path skips covering entire modules.

Because the test runner sets `xfail_strict = true`, a test listed in `NODEIDS_THAT_FAIL` that unexpectedly *passes* is reported as `XPASS` — which is also a failure. You must remove the entry before testing your fix, or you will never see a genuine pass.
The pandas-tests harness runs with `xfail_strict = false` (set in the vendored `pandas-tests/pyproject.toml` to tolerate flaky XPASSes — [rapidsai/cudf#22681](https://github.com/rapidsai/cudf/issues/22681)). A test listed in `NODEIDS_THAT_FAIL` that now *passes* is therefore reported as `XPASS` **without failing the run**, so a stale entry will not flag itself. You must change the false to true yourself before testing your fix; otherwise the test reports `XPASS` instead of a genuine `PASSED` and the dead marker lingers silently. Do not commit this change in any commit.

Search for the node ID:

```bash
grep -n "tests/groupby/test_reductions.py::test_first_last_skipna" \
python/cudf/cudf/pandas/scripts/conftest-patch.py
python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py
```

If found, remove the line. Keys must remain in alphabetical order after the edit. Then validate the file still parses:

```bash
python -c "exec(open('python/cudf/cudf/pandas/scripts/conftest-patch.py').read())"
python -c "exec(open('python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py').read())"
```

If the node ID is *not* found in any dictionary, you are dealing with a new regression — proceed directly to Step 1.
Expand All @@ -92,15 +95,14 @@ If the node ID is *not* found in any dictionary, you are dealing with a new regr
Run from the repo root:

```bash
rm -rf pandas-testing/pandas-tests/
bash python/cudf/cudf/pandas/scripts/run-pandas-tests.sh \
"tests/groupby/test_reductions.py::test_first_last_skipna[Float64-False-False-first]" \
-xvs
```

The script clones the matching pandas version, copies tests, appends the conftest patch, and runs pytest with `-p cudf.pandas`. Substitute your actual node ID.
On the first run the script clones the matching pandas version and copies the test tree (subsequent runs reuse it). It runs pytest with both `-p cudf.pandas` and the marker plugin `-p cudf.pandas.scripts.pandas-testing-plugin`, and automatically applies `-m "not slow and not single_cpu and not db and not network"` and `--disable-warnings`. Substitute your actual node ID.

**If the test passes**: the xfail entry was stale. Commit only the `conftest-patch.py` change and stop.
**If the test passes**: the xfail entry was stale. Commit only the `pandas-testing-plugin.py` change and stop.

**If the test fails**: read the failure output carefully — the assertion message tells you the exact behavioral difference. Proceed to Step 2.

Expand Down Expand Up @@ -327,7 +329,7 @@ bash python/cudf/cudf/pandas/scripts/run-pandas-tests.sh \
"tests/<module_directory>/" --tb=line -q
```

Replace `<module_directory>` with the directory containing your test (e.g. `tests/groupby/`). Any new failures that are not already listed in `conftest-patch.py` must be investigated before committing.
Replace `<module_directory>` with the directory containing your test (e.g. `tests/groupby/`). Any new failures that are not already listed in `pandas-testing-plugin.py` must be investigated before committing.

**d. Add unit tests (where appropriate):**

Expand All @@ -345,7 +347,7 @@ Stage only the intended files — never anything from `pandas-testing/`:

```bash
git add python/cudf/cudf/ # source fix (if applicable)
git add python/cudf/cudf/pandas/scripts/conftest-patch.py # xfail removal
git add python/cudf/cudf/pandas/scripts/pandas-testing-plugin.py # xfail removal
git status # verify nothing from pandas-testing/ is staged
git commit -m "fix(cudf.pandas): <short description>

Expand Down Expand Up @@ -374,13 +376,13 @@ For intentional divergence: stop and ask the user. In most cases, the goal is to

- `mode.pandas_compatible` is automatically set to `True` when `cudf.pandas` is active. This is done at the end of `python/cudf/cudf/pandas/_wrappers/pandas.py`.
- cudf Python is almost entirely pure Python — for inplace installs, changes to `.py` files take effect immediately without rebuilding.
- `pandas-testing/pandas-tests/` must be deleted before each test run to avoid duplicate conftest hook registrations (the script appends the patch file on every run).
- Keys in all three `conftest-patch.py` dictionaries must remain in alphabetical order.
- The xfail/skip markers are applied by a pytest plugin (`-p cudf.pandas.scripts.pandas-testing-plugin`) loaded fresh on every run, not by appending to the pandas `conftest.py`. The runner sets up `pandas-testing/` only when it is missing and never refreshes it afterward, so if you modify `python/cudf/cudf/pandas/scripts/run-pandas-tests.sh` in a way that changes the `pandas-testing/` directory, delete it before re-running the script. Deleting only `pandas-tests/` re-copies the test tree from the existing clone; to pick up a new pandas version delete the whole `pandas-testing/`, since the `pandas/` clone is guarded separately and is not refetched by removing `pandas-tests/` alone.
- Keys in all three `pandas-testing-plugin.py` dictionaries must remain in alphabetical order.
- Never write comments that explain what old code was replaced — write comments about what the code does and why.
- Never modify the pandas test files themselves — fix cudf, not pandas.
- Never fix the testing APIs (like `assert_frame_equal`, `assert_series_equal`) — fix the actual APIs that produce wrong results.
- First see if the problem is in cudf classic and fix it there; if not, then move over to cudf.pandas.
- Tests run with `xfail_strict = true` — a test listed in `NODEIDS_THAT_FAIL` that unexpectedly passes is reported as `XPASS` (also a failure). Remove from the list before testing.
- The pandas-tests harness runs with `xfail_strict = false` (vendored `pandas-tests/pyproject.toml`, to tolerate flaky XPASSes — issue #22681), so a stale `NODEIDS_THAT_FAIL` entry that now passes shows up as a non-failing `XPASS` and won't flag itself. While testing your fix, flip `xfail_strict` to `true` in that file so the `XPASS` surfaces (do not commit that change) and remove the stale entry so the test reports a genuine `PASSED`.
- When a fix requires adding a test dependency, update `dependencies.yaml` (under `test_cudf_pandas_pandas_tests` for conda environments) and run `rapids-dependency-file-generator` to propagate. Never manually edit the generated `pyproject.toml` entries marked as auto-generated.
- Always verify vanilla pandas behavior before implementing proxy-layer fixes. If the test also fails without cudf.pandas, the problem is upstream or environmental, not a cudf bug.
- xfail explanation strings should describe the root cause ("openpyxl limitation", "pandas test assumes xlsxwriter is installed"), not just the error type ("AssertionError", "IndexError").
Loading