Isolate pandas plotting options between sharded tests - #24069
Isolate pandas plotting options between sharded tests#24069galipremsagar wants to merge 3 commits into
Conversation
Preserve and restore pandas plotting options around matplotlib tests so xaxis.compat changes cannot leak across tests on a shard worker. Remove 57 resulting shard skips and cover cleanup after successful and failed tests under pandas and cudf.pandas.
|
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 (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 9 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe pandas testing plugin now restores plotting options after tests that use `mpl_cleanup. New pytester coverage checks isolation across acceleration and failure scenarios. Sharded skip entries are narrowed. ChangesPlotting isolation
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change isolates pandas plotting settings between tests and removes obsolete sharded skips. Coverage includes successful and failing tests with and without cudf.pandas acceleration, with no remaining merge-readiness risk identified. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (1 skipped: 1 too large.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
/okay to test 4600a37 |
|
/okay to test 0db0dcd |
Description
Fix the plotting-state group in
NODEIDS_TO_SKIP_WHEN_SHARDED(#22992).test_xcompat_plot_paramsleaves the public pandas plotting optionxaxis.compatenabled. An adjacent test normally resets it, but hashing those tests into different shards separates the mutation and reset. Later time-series plots then take the wrong plotting path. The same minimal polluter/victim pair fails with vanilla pandas, so this is test isolation, not a GPU plotting implementation bug.Snapshot and restore pandas plotting options around tests using the existing
mpl_cleanupfixture, including when a test fails. Remove the 57 shard-only skips reproduced by this leak. Four independent figure-pickling skips are handled separately.Validation
Plotting intentionally executes through pandas/Matplotlib. This changes only test-state isolation; disabling CPU fallback is not applicable to this group. Tests used pandas 3.0.3 and local source Python with existing cuDF 26.10 native libraries, plus an external compatibility hook for the exact
apply_boolean_mask→apply_retention_maskGPU API rename. No installed environment or vendored pandas tests were modified. Matching main native-library validation remains for CI.