Convert conftest-patch.py to a pytest plugin for pandas testing - #22643
Conversation
|
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. |
|
/ok to test 429b4ad |
|
/ok to test a549ef8 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR refactors pandas test warning suppression by moving helper logic from the pytest plugin into a wrapper module, updating the plugin to properly register as a pytest hook, and configuring test runners to load the new plugin directly instead of patching conftest files. ChangesPandas Test Warning Suppression
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@python/cudf/cudf/pandas/_wrappers/pandas.py`:
- Around line 1512-1536: Add unit tests and a benchmark for the new proxy helper
functions: null_assert_produces_warning, _register_assert_produces_warning,
null_raises_chained_assignment_error, and
_register_raises_chained_assignment_error. Create pytest cases that verify each
helper returns a contextmanager, that using the context manager
suppresses/handles pandas warnings and chained-assignment errors as intended
(exercise both direct calls to null_* helpers and the registered proxy wrappers
_register_*), and assert proper enter/exit behavior; also add a lightweight perf
benchmark (pytest-benchmark or project benchmark style) that measures
entering/exiting these context managers to satisfy repo policy. Ensure tests
live under the repo's test pattern and include necessary imports/mocks for
pd._testing so the proxy registration is exercised.
- Around line 1512-1520: The current null_assert_produces_warning context
manager yields an empty list and doesn't capture warnings, so under
filterwarnings("error")/ -Werror warnings still raise; modify
null_assert_produces_warning to use warnings.catch_warnings(record=True) and set
a permissive filter (e.g., warnings.simplefilter("always") or "default") inside
the context so warnings are recorded rather than turned into errors, then yield
the recorded list and restore warning state on exit; update the function
null_assert_produces_warning to import and use the warnings module and return
the captured warnings list similarly to pandas.assert_produces_warning.
🪄 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: 231faff8-bf1e-48df-bd51-acdacba82586
📒 Files selected for processing (5)
ci/cudf_pandas_scripts/pandas-tests/run.shpyproject.tomlpython/cudf/cudf/pandas/_wrappers/pandas.pypython/cudf/cudf/pandas/scripts/pandas-testing-plugin.pypython/cudf/cudf/pandas/scripts/run-pandas-tests.sh
💤 Files with no reviewable changes (2)
- ci/cudf_pandas_scripts/pandas-tests/run.sh
- pyproject.toml
| def null_assert_produces_warning(*args, **kwargs): | ||
| # We do not want pandas unit tests to fail because | ||
| # assert_produces_warning doesn't see a warning. | ||
| # No an explicit public API |
There was a problem hiding this comment.
What does this comment mean?
There was a problem hiding this comment.
I suppose I was trying to convey "this is a proxy that doesn't faithfully fall back to pandas because we need it to be a no-op when we run pandas tests". I can tune this comment in a follow up PR or a next commit if needed.
|
/merge |
Description
This PR converts and renames
conftest-patch.pyto apandas-testing-plugin.pypytest plugin file that can be used as pytest -p pandas-testing-plugin to patch pandas testing utilities and dynamically add our xfail or skip markers instead of modifying the existing pandas' conftest.py.Hoping to reduce the modifications we make to the cloned repo in a follow up PR
Additionally moves some flags set in CI scripts to the script run locally for better CI/local alignment
Checklist