Skip to content

[python-package] Remove dead Python 2 guard and sklearn.cross_validation fallback#12086

Open
PavelGuzenfeld wants to merge 6 commits intodmlc:masterfrom
thebandofficial:fix/remove-dead-compat-code
Open

[python-package] Remove dead Python 2 guard and sklearn.cross_validation fallback#12086
PavelGuzenfeld wants to merge 6 commits intodmlc:masterfrom
thebandofficial:fix/remove-dead-compat-code

Conversation

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor

Summary

Remove two pieces of dead compatibility code from compat.py:

  • assert sys.version_info[0] == 3 — XGBoost requires Python >= 3.10
    (pyproject.toml), so this check can never fail. Additionally, using
    assert for version checks is incorrect since assertions are stripped
    under python -O.

  • sklearn.cross_validation import fallbacksklearn.cross_validation
    was removed in scikit-learn 0.20 (2018). Any scikit-learn version that
    supports Python >= 3.10 will have sklearn.model_selection, making
    the try/except fallback unreachable.

Files changed

  • python-package/xgboost/compat.py — 7 lines removed, 1 added

- Remove `assert sys.version_info[0] == 3` — XGBoost requires
  Python >= 3.10 (pyproject.toml), making this check dead code.
  Using `assert` for version checks is also incorrect as assertions
  are stripped under `python -O`.

- Remove `sklearn.cross_validation` import fallback —
  `sklearn.cross_validation` was removed in scikit-learn 0.20 (2018).
  Any scikit-learn version supporting Python 3.10+ will have
  `sklearn.model_selection`.
@RAMitchell RAMitchell requested a review from Copilot March 16, 2026 08:36
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes unreachable legacy compatibility code in compat.py now that XGBoost requires modern Python/scikit-learn versions.

Changes:

  • Removed a dead Python 2 guard that could never trigger under the project’s Python >= 3.10 requirement.
  • Removed an unreachable fallback import of sklearn.cross_validation.StratifiedKFold, importing only from sklearn.model_selection.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@RAMitchell
Copy link
Copy Markdown
Member

Please address CI failures

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

Fixed the pre-commit failure — ruff flagged two formatting issues:

  • Missing blank line before def py_str (need 2 blank lines before top-level functions)
  • Extra blank line between RegressorMixin and StratifiedKFold imports

The Windows test failure was unrelated (self-hosted runner lost connectivity).

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

The only CI failure — Google Test (C++) CUDA 12 (mgpu) — is unrelated to this PR's changes. It failed during Docker container initialization due to an ECR authentication error:

Docker login for '492475357299.dkr.ecr.us-west-2.amazonaws.com' failed with exit code 1

This is a transient infrastructure issue (AWS ECR auth), not a code problem. A maintainer with admin access would need to re-trigger the failed job.

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

Hi, friendly ping on this PR. It has been approved by @RAMitchell and all CI checks are passing except for a single infrastructure-related failure (AWS ECR Docker login on the CUDA mgpu job), which appears unrelated to the changes here. I attempted to re-run the failed job but don't have the permissions. Could a maintainer re-trigger that job or merge given the failure is infrastructure-related? Thank you!

@RAMitchell
Copy link
Copy Markdown
Member

RAMitchell commented Mar 19, 2026

I prepared a fix for the macOS x86_64 wheel CI failure here: RAMitchell#2

It avoids pulling Homebrew LLVMs libunwind into the wheel repair step by preferring Condas llvm-openmp runtime for macOS wheel builds, preserving the current macOS 10.15 deployment target instead of bumping it to 14.0.

If useful, the fix commit is caa714533c0503ca7e90d73339557314908a5341.

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

Thanks @RAMitchell! I've cherry-picked your fix commit (caa7145) into this branch to resolve the macOS x86_64 wheel CI failure. Let's see if CI passes now.

@PavelGuzenfeld
Copy link
Copy Markdown
Contributor Author

Thanks @RAMitchell! I've cherry-picked your fix commit (caa7145) into this branch to resolve the macOS x86_64 wheel CI failure. Let's see if CI passes now.

Issue persists.

@RAMitchell
Copy link
Copy Markdown
Member

We will try to fix it in #12108

PavelGuzenfeld added a commit to PavelGuzenfeld/xgboost that referenced this pull request Mar 19, 2026
Add section documenting 3 merged PRs (dmlc#12087, dmlc#12089, dmlc#12094),
1 open PR (dmlc#12086), and 3 closed/superseded PRs.
@trivialfis
Copy link
Copy Markdown
Member

Out of curiosity, is there something like the previous k price happening? Looking at your agent tracker with the difficulty classification, looks quite similar to the trend in that competition.

@RAMitchell
Copy link
Copy Markdown
Member

Please remove bde4a3e (macos fix) and we will merge it.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants