-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ci] [python-package] [R-package] adapt to scikit-learn check_sample_weight_equivalence changes, stop testing against R 3.6 on Linux #6733
Conversation
Noticing that the R 3.6 CI job has been failing for a couple days like this:
That's happening here: LightGBM/.github/workflows/r_package.yml Lines 176 to 180 in 83c0ff3
Will have to fix that in this PR too. I'll try that. Related prior conversations:
|
compiler: gcc | ||
r_version: 3.6 | ||
build_type: cmake | ||
container: 'ubuntu:18.04' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to fix #6733 (comment), it seems that the workaround from actions/checkout#1590 (comment) is no longer working.
See actions/checkout#1590 (comment).
Between this and all the work that was required with package-pinning in #6718, I think we should just fully stop testing R 3.6 on Linux in CI here. As we did for macOS in #5859.
Note that we'll still have test coverage on Windows:
LightGBM/.github/workflows/r_package.yml
Lines 81 to 87 in 83c0ff3
- os: windows-latest | |
task: r-package | |
compiler: MINGW | |
toolchain: MINGW | |
r_version: 3.6 | |
build_type: cmake | |
container: null |
LightGBM/.github/workflows/r_package.yml
Lines 95 to 102 in 83c0ff3
# Visual Studio 2019 | |
- os: windows-2019 | |
task: r-package | |
compiler: MSVC | |
toolchain: MSVC | |
r_version: 3.6 | |
build_type: cmake | |
container: null |
Dropping this testing gets us the following:
- reduces CI configuration complexity (see all the code removed in this PR)
- allows upgrading to
actions/checkout@v4
(no more skipping PRs like [ci]: Bump actions/checkout from 3 to 4 in the ci-dependencies group #6712) - reduced ongoing maintenance burden, via avoiding things like these:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you very much for quick fix! Nice to see this simplification diff for R.
Thanks very much for the quick review! Merging this to unblock CI. |
More changes to estimator checks have been made in
scikit-learn
, and those broke CI here.CI is failing like this when using the latest
scikit-learn
nightlies:(build link)
Looks like scikit-learn/scikit-learn#30137 removed
check_sample_weight_equivalence
, in favor of 2 new functions namescheck_sample_weight_equivalence_on_{dense,sparse}_data
. This updateslightgbm
's xfail_checks tag to include all 3 names, so those tests are correctly xfail'd for the full range ofscikit-learn
versionslightgbm
supports.update: this also removes the R 3.6 Linux CI job to fix another blocking CI issue, see #6733 (comment)